[systemd-devel] [PATCH] Set mac address in link initialization

[Dave Reisner]

On Wed, Jun 04, 2014 at 08:22:11PM +0200, Lennart Poettering wrote:
> On Wed, 04.06.14 13:48, Dave Reisner (dreisner at archlinux.org) wrote:
> 
> > 505f8da7325 left link->mac uninitialized, causing MACAddress based
> > [Match] sections to fail to match anything.
> > 
> > https://bugs.freedesktop.org/show_bug.cgi?id=79638
> > ---
> >  src/network/networkd-link.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
> > index 6e6fb83..f3bab9f 100644
> > --- a/src/network/networkd-link.c
> > +++ b/src/network/networkd-link.c
> > @@ -1683,8 +1683,18 @@ int link_initialized(Link *link, struct udev_device *device) {
> >          if (link->state != LINK_STATE_INITIALIZING)
> >                  return 0;
> >  
> > -        if (device)
> > +        if (device) {
> > +                const char *mac_address = udev_device_get_sysattr_value(
> > +                                device, "address");
> 
> Please avoid declaring a variable and invoking a function within the
> same expression.
> 
> It's fine to initialize a variable while declaring it, but please only
> whith a constant value or macro or something, but don't mix variable
> declarations with function invocations so much. It doesn't help
> readability...
> 

Sure, done.

> > +
> > +                if (mac_address) {
> > +                        struct ether_addr *hwaddr = ether_aton(mac_address);

Fixed this, too.

> > +                        if (hwaddr)
> > +                                memcpy(&link->mac, hwaddr, sizeof(struct ether_addr));
> > +                }
> > +
> >                  link->udev_device = udev_device_ref(device);
> > +        }
> >  
> >          log_debug_link(link, "udev initialized link");
> >  
> 
> 
> Lennart
> 
> -- 
> Lennart Poettering, Red Hat