Comment on svn 16066

[Denis Vlasenko]

Hello Rob,

Please include my address in To: or CC:,
I sometimes miss mails which are sent to list only.
Thanks.

On Thursday 07 September 2006 19:59, Rob Landley wrote:
> +/* Something is definitely wrong here. IPv4 addresses
> + * in variables of type long?? BTW, we use inet_ntoa()
> + * in the code. Manpage says that struct in_addr has a member of type long 
> (!)
> + * which holds IPv4 address, and the struct is passed by value (!!)
> + */
> 
> Why is having an ipv4 address in a long confusing?  You're worried that on 
> 64-bit systems it's wasting 4 bytes, and should thus be an "int" instead?
>
> I've read the comment but I'm unclear on the objection.
 
I think that it should be unit32_t in all places which
genuinely are ipv4_only-specific. I don't know whether dhcp
is even exists in ipv6 world, so maybe here it is ok.

I added a comment and did those small fixes in order to
close a bug.

> +                  server_addr = *(uint32_t*)temp;
> 
> That sort of typecast-and-dereference breaks strict aliasing, and will thus 
> generate a warning.

It's dirty. temp is a uint8_t* returned from get_option(&packet, DHCP_SERVER_ID).
We know that it points to binary ip address in network byteorder.
server_addr, where we want to stuff address,
is used to populate variables of type "struct in_addr",
which contain ip address in network byteorder too (man inet_ntoa says so).

Thus server_addr = *(uint32_t*)temp looks okay as a quick fix.

Looking into the entire mess in order to figure out whether
it is sane would be better still, but I have no such plans
currently...

How should it be done to silence your gcc?

(1) Maybe by placing cast right before get_option(...)?
(2) byte-at-a-time "my CPU cannot dereference words with
    odd address" style?

> Although I can't confirm that because I hit an earlier  
> warning, and we've have -Werror:
> 
>   CC networking/udhcp/dhcpc.o
> cc1: warnings being treated as errors
> /home/landley/busybox/busybox/networking/udhcp/dhcpc.c: In 
> function ‘udhcpc_main’:
> /home/landley/busybox/busybox/networking/udhcp/dhcpc.c:147: warning: ‘lease’ 
> may be used uninitialized in this function
> make[1]: *** [/home/landley/busybox/busybox/networking/udhcp/dhcpc.o] Error 1
> make: *** [_all] Error 2
> 
> What compiler version are you test-building these on before checking them in?

gcc 3.4.3 and sometimes gcc 3.4.6. I do allyesconfig builds before commits.
--
vda