[systemd-devel] [PATCH 21/28] dhcp: Handle received DHCP Offer message

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.flykt at linux.intel.com) wrote:

> +        if (client->fd > 0)
> +                close(client->fd);
> +        client->fd = 0;

fd 0 is a valid fd (actually refers to stdin), I am pretty sure you want
to assign -1 here, no?

> +        if (hdrlen + ntohs(offer->udp.len) > len ||
> +            client_checksum(&offer->ip.ttl, ntohs(offer->udp.len) + 12))
> +                return -EINVAL;

Hmm, so it might make sense to use betoh16() and friends instead of
ntohs() here, where it makes sense, and to declare the integers as
be16_t, be32_t and so on. This is defined in "sparse-endian.h" and is
useful for using the kernel sparse checker on this code, and it will
then detect whenever a non-native endian integer is accessed without
conversion.

> +        lease = new0(DHCPLease, 1);
> +        if (!lease)
> +                return -ENOBUFS;

ENOMEM please

> +
> +error:
> +        free(lease);
> +
> +        return -ENOMSG;
> +}

_cleanup_free_!

> +
> +static int client_receive_raw_message(sd_event_source *s, int fd,
> +                                      uint32_t revents, void *userdata)
> +{
> +        DHCPClient *client = userdata;
> +        int err = 0;
> +        int len, buflen;
> +        uint8_t *buf;
> +        DHCPPacket *message;
> +
> +        buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
> +        buf = malloc0(buflen);
> +        if (!buf)
> +                goto error;

Hmm, you eat up the rror here, is that intended?

> +        len = read(fd, buf, buflen);
> +        if (len < 0) {
> +                err = -errno;
> +                goto error;
> +        }
> +
> +        message = (DHCPPacket *)buf;
> +
> +        switch (client->state) {
> +        case DHCP_STATE_SELECTING:
> +
> +                if (client_receive_offer(client, message, len) >= 0) {
> +
> +                        close(client->fd);
> +                        client->fd = 0;

-1 again...

> +error:
> +        if (!err)
> +                read(fd, buf, 1);

You forget to free buf here...

Lennart

-- 
Lennart Poettering, Red Hat