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