On Fri, 2014-10-03 at 15:48 +0300, Patrik Flykt wrote:
> On Fri, 2014-09-26 at 15:15 -0500, Dan Williams wrote:
> >          /* RFC2132 section 4.1.1:
> >             The client MUST include its hardware address in the ’chaddr’ 
> > field, if
> > -           necessary for delivery of DHCP reply messages.
> > +           necessary for delivery of DHCP reply messages.  Non-Ethernet
> > +           interfaces will leave 'chaddr' empty and use the client 
> > identifier
> > +           instead (eg, RFC 4390 section 2.1).
> >           */
> > -        memcpy(&packet->dhcp.chaddr, &client->client_id.mac_addr, 
> > ETH_ALEN);
> > +        if (client->mac_addr_len == ETH_ALEN)
> > +                memcpy(&packet->dhcp.chaddr, &client->mac_addr, ETH_ALEN);
> 
> Sorry about the late review, but shouldn't this be more generic if
> written:
>         if (client->mac_addr_len)
>                 memcpy(&packet->dhcp.chaddr, &client->mac_addr, 
> client->mac_addr_len);
> 
> With that, all cases are covered. Which, AFAIK, are none in addition to
> ethernet. An assert() should check the given MAC address length at some
> point in the code so that it cannot be > 16.

And then I notice that infiniband has a MAC address length of 20. For
all practical purposes this works as expected. Except if we want to go
nitpicking when setting the MAC address and also require the address
type to be supplied? Probably not...

Cheers,

        Patrik


_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to