[lwip-devel] X8_F and other formatting

[Leon Woestenberg]

Hello,

1) Now that we have X8_F, why not also introduce U8_F and get rid of this:

/* Get one byte from the 4-byte address */
#define ip4_addr1(ipaddr) (((u8_t*)(ipaddr))[0])
#define ip4_addr2(ipaddr) (((u8_t*)(ipaddr))[1])
#define ip4_addr3(ipaddr) (((u8_t*)(ipaddr))[2])
#define ip4_addr4(ipaddr) (((u8_t*)(ipaddr))[3])
/* These are cast to u16_t, with the intent that they are often arguments
 * to printf using the U16_F format from cc.h. */
#define ip4_addr1_16(ipaddr) ((u16_t)ip4_addr1(ipaddr))
#define ip4_addr2_16(ipaddr) ((u16_t)ip4_addr2(ipaddr))
#define ip4_addr3_16(ipaddr) ((u16_t)ip4_addr3(ipaddr))
#define ip4_addr4_16(ipaddr) ((u16_t)ip4_addr4(ipaddr))

I do not see any benefit in these macro's. I expected them to be
endianess aware or something, but they only obfuscate the code IMHO.

2) in arch.h, let's define the default formatter values, so that
people see what the length modifier is. Without it, it's hard to spot
bugs.

Also, I found a memory corruption bug here related to X8_F usage:

#ifndef X8_F
#define X8_F  "02x"
#endif /* X8_F */

    ("ethernet_input:
dest:%"X8_F":%"X8_F":%"X8_F":%"X8_F":%"X8_F":%"X8_F",
src:%"X8_F":%"X8_F":%"X8_F":%"X8_F":%"X8_F":%"X8_F", type:%"X16_F"\n",
     (unsigned)ethhdr->dest.addr[0], (unsigned)ethhdr->dest.addr[1],
(unsigned)ethhdr->dest.addr[2],
     (unsigned)ethhdr->dest.addr[3], (unsigned)ethhdr->dest.addr[4],
(unsigned)ethhdr->dest.addr[5],
     (unsigned)ethhdr->src.addr[0], (unsigned)ethhdr->src.addr[1],
(unsigned)ethhdr->src.addr[2],
     (unsigned)ethhdr->src.addr[3], (unsigned)ethhdr->src.addr[4],
(unsigned)ethhdr->src.addr[5],
     (unsigned)htons(ethhdr->type)));

I'm submitting the fix for that.

3) if we want to go ipv4/ipv6 transparent in our macro's (wherever
possible) I think we must use ip(6)_addr_debug_print wherever
possible.


Regards,
-- 
Leon