[systemd-devel] [PATCH v2 20/26] dhcp: Handle received DHCP Offer message

[Lennart Poettering]

On Wed, 27.11.13 17:13, Patrik Flykt (Patrik.Flykt at linux.intel.com) wrote:

> > > +        buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
> > > +        buf = malloc0(buflen);
> > > +        if (!buf) {
> > > +                read(fd, &tmp, 1);
> > > +                return 0;
> > > +        }
> > 
> > Hmm, how long is this message? Given that you don't keep the memory
> > around, why not just allocate this on the stack? Either with alloca, or
> > even directly as an uint8_t array? Given that this seems to be
> > relatively short, this should not be an issue and is one error source
> > less...
> 
> This one would be the minimum IPv4 packet size, 576 bytes. Should I go
> with alloca() here?

As a rule of thumb I'd alway prefer stack allocation for things < 2K or
so... In userspace that should be totally OK, and if you never need this
buffer outside of this call, then stack allocation should be fine.

Note that allocating this as explicit array on the stack is usually the
better option than alloca(), since alloca() can get very confusing when
used inside of loops.

Hence:

       uint8_t buf[sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE]; 

sounds better, if you can avoid:

       void *buf;
       buf = alloca0(sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE);

which is still better than:

       _cleanup_free_ void *buf;
       buf = malloc0(sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE);
       if (!buf)
               ...

All of course only if the the size is not huge, and if you never need
the thing outside of the local scope...

Lennart

-- 
Lennart Poettering, Red Hat