Syslinux - Jun 2015 - [syslinux:master] core/pxe: extend parse_dhcp() for packet type

On 06/21/2015 11:12 AM, syslinux-bot for Gene Cumm
wrote:
> Commit-ID:  38e861ebf45a804bc5fbd74d9c19292822c84487
> Gitweb:    
http://www.syslinux.org/commit/38e861ebf45a804bc5fbd74d9c19292822c84487
> Author:     Gene Cumm <gene.cumm at gmail.com>
> AuthorDate: Sat, 20 Jun 2015 21:17:18 -0400
> Committer:  Gene Cumm <gene.cumm at gmail.com>
> CommitDate: Sat, 20 Jun 2015 21:17:18 -0400
> 
> core/pxe: extend parse_dhcp() for packet type
> 
> Add packet type so we can eventually only grab certain data elements from
> the DHCP packets appropriately
> 
> Signed-off-by: Gene Cumm <gene.cumm at gmail.com>
>   *
> + * pkt_type 1 for Discover, 2 for Ack, 3 for ProxyDHCP, 0 for everything
> + *
What does 0 even mean here?  It makes the code more complicated, and it
is never used, and it is not at all clear to me that it is even meaningful.

	-hpa

On Mon, Jun 22, 2015 at 4:18 AM, H. Peter Anvin <hpa at zytor.com>
wrote:
> On 06/21/2015 11:12 AM, syslinux-bot for Gene Cumm wrote:
>> Commit-ID:  38e861ebf45a804bc5fbd74d9c19292822c84487
>> Gitweb:    
http://www.syslinux.org/commit/38e861ebf45a804bc5fbd74d9c19292822c84487
>> Author:     Gene Cumm <gene.cumm at gmail.com>
>> AuthorDate: Sat, 20 Jun 2015 21:17:18 -0400
>> Committer:  Gene Cumm <gene.cumm at gmail.com>
>> CommitDate: Sat, 20 Jun 2015 21:17:18 -0400
>>
>> core/pxe: extend parse_dhcp() for packet type
>>
>> Add packet type so we can eventually only grab certain data elements
from
>> the DHCP packets
appropriatelyhttps://mail.google.com/mail/u/0/#inbox/14e17521f4b19a58
>>
>> Signed-off-by: Gene Cumm <gene.cumm at gmail.com>
>
>>   *
>> + * pkt_type 1 for Discover, 2 for Ack, 3 for ProxyDHCP, 0 for
everything
>> + *
>
> What does 0 even mean here?  It makes the code more complicated, and it
> is never used, and it is not at all clear to me that it is even meaningful.
>
>         -hpa
>
Old behavior; process everything in a packet.  I see your point and
agree now it's not necessary for parse_dhcp().  It's an internal-only
function and all callers are covered.

I was also considering passing pkt_type to parse_dhcp_option() in
which case 0 has meaning for the before and after hard-coded options
as any arbitrary option could be hard-coded.  Passing pkt_type to
parse_dhcp_option() would allow us to skip options that shouldn't be
seen in a given packet type (request, ack, proxyDHCP, PXE-reply)

-- 
-Gene