[PATCH] rsl: fix the unaligned memory access

Harald Welte laforge at gnumonks.org
Thu Apr 18 14:27:56 UTC 2013


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 at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)




More information about the OpenBSC mailing list