Hi Nicolas,
thanks for your patch.
On Thu, Apr 18, 2013 at 08:25:31AM -0400, Nicolas J. Bouliane wrote:
- connect_ip = (uint32_t *) TLVP_VAL(&tp,
RSL_IE_IPAC_REMOTE_IP);
- connect_port = (uint16_t *) TLVP_VAL(&tp, RSL_IE_IPAC_REMOTE_PORT);
+
+ if (TLVP_PRESENT(&tp, RSL_IE_IPAC_REMOTE_IP)) {
+ connect_ip = tlvp_val32_unal(&tp, RSL_IE_IPAC_REMOTE_IP);
+ }
+
+ if (TLVP_PRESENT(&tp, RSL_IE_IPAC_REMOTE_PORT)) {
+ connect_port = tlvp_val16_unal(&tp, RSL_IE_IPAC_REMOTE_PORT);
+ }
Several comments for that:
1) we unconditionally assume that the IEs are present in the original
code. This is most likely due to the fact that they are specified to
be there. If they don't exist, we fail miserably. With your new
code, we still fail in strange ways. Best would be to print a
LOGL_WARN or ERROR message about missing information elements.
2) kernel coding style (which we use universally in our projects) says:
If there's only a single statement after an if statement, we don't
use curly braces.
I'd appreciate if you could submit a new patch (replacing the original
patch, not an incremental one) to that regard. Thanks.
Regards,
Harald
--
- Harald Welte <laforge(a)gnumonks.org>
http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)