[systemd-devel] [PATCH 02/28] dhcp: Add DHCP client initialization

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 13, 2013 at 11:22:30PM +0200, Patrik Flykt wrote:
> Provide functionality for initializing a DHCP client struct, setting
> interface index, last used address and additional options to request.
> On initialization the most useful options are added by default.
> ---
>  src/dhcp/client.c |  137 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/dhcp/client.h |   34 +++++++++++++
>  2 files changed, 171 insertions(+)
>  create mode 100644 src/dhcp/client.c
>  create mode 100644 src/dhcp/client.h
> 
> diff --git a/src/dhcp/client.c b/src/dhcp/client.c
> new file mode 100644
> index 0000000..172ddd9
> --- /dev/null
> +++ b/src/dhcp/client.c
> @@ -0,0 +1,137 @@
> +/***
> +  This file is part of systemd.
> +
> +  Copyright (C) 2013 Intel Corporation. All rights reserved.
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <stdio.h>
> +
> +#include "util.h"
> +#include "list.h"
> +
> +#include "protocol.h"
> +#include "client.h"
> +
> +struct DHCPClient {
> +        DHCPState state;
> +        int index;
> +        uint8_t *req_opts;
> +        int req_opts_size;
> +        struct in_addr *last_addr;
> +};
> +
> +static uint8_t default_req_opts[] = {
> +        DHCP_OPTION_SUBNET_MASK,
> +        DHCP_OPTION_ROUTER,
> +        DHCP_OPTION_HOST_NAME,
> +        DHCP_OPTION_DOMAIN_NAME,
> +        DHCP_OPTION_DOMAIN_NAME_SERVER,
> +        DHCP_OPTION_NTP_SERVER,
> +};
> +
> +int dhcp_client_set_request_option(DHCPClient *client, uint8_t option)
> +{
> +        int i;
> +
> +        if (!client)
> +                return -EINVAL;
> +
> +        if (client->state != DHCP_STATE_INIT)
> +                return -EBUSY;
> +
> +        switch(option) {
> +        case DHCP_OPTION_PAD:
> +        case DHCP_OPTION_OVERLOAD:
> +        case DHCP_OPTION_MESSAGE_TYPE:
> +        case DHCP_OPTION_PARAMETER_REQUEST_LIST:
> +        case DHCP_OPTION_END:
> +                return -EINVAL;
> +
> +        default:
> +                break;
> +        }
> +
> +        for (i = 0; i < client->req_opts_size; i++)
> +                if (client->req_opts[i] == option)
> +                        return -EEXIST;
> +
> +        client->req_opts_size++;
> +        client->req_opts = realloc(client->req_opts, client->req_opts_size);
> +        if (!client->req_opts) {
> +                client->req_opts_size = 0;
> +                return -ENOBUFS;
> +        }
Hm, you have memory leak here. And repeated realloc is slow.

Try something like this:
           if (!GREEDY_REALLOC(client->req_opts,
	                       client->req_opts_size,
                               client->req_opts_size + 1))
                   return -ENOMEM;

In general we use -ENOMEM, so for consistency it should be used here too. 

> +        client->req_opts[client->req_opts_size - 1] = option;
> +
> +        return 0;
> +}
> +
> +int dhcp_client_set_request_address(DHCPClient *client,
> +                struct in_addr *last_addr)
> +{
> +        if (!client)
> +                return -EINVAL;
> +
> +        if (client->state != DHCP_STATE_INIT)
> +                return -EBUSY;
> +

> +        client->last_addr = malloc(sizeof(struct in_addr));
> +        if (!client->last_addr)
> +                return -ENOBUFS;
> +
> +        memcpy(&client->last_addr, last_addr, sizeof(struct in_addr));
client->last_addr = memdup(last_addr, sizeof(struct in_addr));

> +
> +        return 0;
> +}
> +
> +int dhcp_client_set_index(DHCPClient *client, int interface_index)
> +{
> +        if (!client || interface_index < -1)
> +                return -EINVAL;
> +
> +        if (client->state != DHCP_STATE_INIT)
> +                return -EBUSY;
> +
> +        client->index = interface_index;
> +
> +        return 0;
> +}
> +
> +DHCPClient *dhcp_client_new(void)
> +{
> +        DHCPClient *client;
> +        int i;
> +
> +        client = new0(DHCPClient, 1);
> +        if (!client)
> +                return NULL;
> +
> +        client->state = DHCP_STATE_INIT;
> +        client->index = -1;
> +
> +        client->req_opts_size = sizeof(default_req_opts)
> +                / sizeof(default_req_opts[0]);
ELEMENTSOF(default_req_opts)

> +        client->req_opts = malloc(client->req_opts_size);
> +        for (i = 0; i < client->req_opts_size; i++)
> +                client->req_opts[i] = default_req_opts[i];

client->req_opts = memdup_multiply(client->req_opts_size, sizeof(default_req_opts),
		                    client->req_opts_size); 

> +
> +        return client;
> +}
> diff --git a/src/dhcp/client.h b/src/dhcp/client.h
> new file mode 100644
> index 0000000..447c03b
> --- /dev/null
> +++ b/src/dhcp/client.h
> @@ -0,0 +1,34 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +#pragma once
> +
> +/***
> +  This file is part of systemd.
> +
> +  Copyright (C) 2013 Intel Corporation. All rights reserved.
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +#include <netinet/in.h>
> +
> +struct DHCPClient;
> +typedef struct DHCPClient DHCPClient;
> +
> +int dhcp_client_set_request_option(DHCPClient *client, uint8_t option);
> +int dhcp_client_set_request_address(DHCPClient *client,
> +                                    struct in_addr *last_address);
> +int dhcp_client_set_index(DHCPClient *client, int interface_index);
> +
> +DHCPClient *dhcp_client_new(void);

Zbyszek