[U-Boot] [PATCH 1/1] net:Fix potential empty DHCP Parameter Request List

[Jason Liu]

2010/10/23 Mike Frysinger <vapier at gentoo.org>:
> On Friday, October 22, 2010 04:36:25 Jason Liu wrote:
>> From: Gray Remlin <g_remlin at rocketmail.com>
>>
>> Can't get IP address with dhcp due to the dhcp server not
>> allow the empty param list request under some network env
>>
>> --- a/net/bootp.c
>> +++ b/net/bootp.c
>> @@ -417,9 +417,19 @@ static int DhcpExtended (u8 * e, int message_type,
>> IPaddr_t ServerID, IPaddr_t R return x - start;
>>  #endif
>>
>> +#if defined(CONFIG_BOOTP_SUBNETMASK) || \
>> +    defined(CONFIG_BOOTP_TIMEOFFSET) || \
>> +    defined(CONFIG_BOOTP_GATEWAY) || \
>> +    defined(CONFIG_BOOTP_DNS) || \
>> +    defined(CONFIG_BOOTP_HOSTNAME) || \
>> +    defined(CONFIG_BOOTP_BOOTFILESIZE) || \
>> +    defined(CONFIG_BOOTP_BOOTPATH) || \
>> +    defined(CONFIG_BOOTP_NISDOMAIN) || \
>> +    defined(CONFIG_BOOTP_NTPSERVER)
>>       *e++ = 55;              /* Parameter Request List */
>>        cnt = e++;             /* Pointer to count of requested items */
>>       *cnt = 0;
>> +#endif
>
> this list is pretty ugly and prone to breakage.  how about having the code
> back itself up and let gcc optimize things away ?  two ways of doing this ...
>
> (1) after the current ifdef list and before the "*e++ = 255;", add like:
>        /* no options, so back up to avoid sending an empty request list */
>        if (*cnt == 0)
>                e -= 2;
>
> (2) add a "bool empty_list" to this func.  where we set "*cnt = 0", do:
>        empty_list = true;
> then in every ifdef currently, add:
>        empty_list = false;
> and at the end, do:
>        /* no options, so back up to avoid sending an empty request list */
>        if (empty_list)
>                e -= 2;

Good point, thanks, mike. I will select option 1 to send v2 patch soon.

> -mike
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>