Wed 29 Oct 2014 06:01:02 PM UTC, comment #2:
> not exactly (as far as I understood it) : the calls to the functions (from etharp.c, and probably only there) don't have to change, and would then be correctly self-documented.
> See (etharp.c@439)
> ETHADDR32_COPY(ðhdr->dest, dst);
> ETHADDR16_COPY(ðhdr->src, src);
> which expands to
> SMEMCPY(ðhdr->dest, dst , ETHARP_HWADDR_LEN);
> SMEMCPY(ðhdr->src, src , ETHARP_HWADDR_LEN);
> which expands to
> memcpy(ðhdr->dest, dst , ETHARP_HWADDR_LEN);
> memcpy(ðhdr->src, src , ETHARP_HWADDR_LEN);
>
> The copy is done from the right side variable onto the left side variable anyway, so I don't see why the macro
> #define ETHADDR32_COPY(src, dst) SMEMCPY(src, dst, ETHARP_HWADDR_LEN)
> names the destination src and the source dst.
> Even if it does work...
>
> No API change is needed anywhere if you swap BOTH src and dst names. Just the "self documentation" (= this h file) is fixed.
> As far as I know ;-) And it worked on my target.
> Except if I wrote my bug report wrong, I'm not suggesting to change only one of the dst,src pair, which would indeed force all user code to
Indeed, this wasn't clear at first for me this was only about a cosmetic change.
Fixed, thank you ! :)
|
Wed 29 Oct 2014 05:29:45 PM UTC, original submission:
Hi
In file src/include/netif/etharp.h, there are the macros
#define ETHADDR32_COPY(src, dst) SMEMCPY(src, dst, ETHARP_HWADDR_LEN)
#define ETHADDR16_COPY(src, dst) SMEMCPY(src, dst, ETHARP_HWADDR_LEN)
It works fine, but in reality all memcpy() functions I ever saw actually use (dst, src), as does the one on my ARM board, called after expanding all macros.
So it's easy to mess up a port or not understand what the code is supposed to do with this wrong argument order.
Can it be fixed to the following ?
#define ETHADDR32_COPY(dst, src) SMEMCPY(dst, src, ETHARP_HWADDR_LEN)
#define ETHADDR16_COPY(dst, src) SMEMCPY(dst, src, ETHARP_HWADDR_LEN)
B.G.
|