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

[Lennart Poettering]

On Mon, 25.11.13 09:13, Patrik Flykt (patrik.flykt at linux.intel.com) wrote:

> +
> +DHCPClient *sd_dhcp_client_new(void)
> +{
> +        DHCPClient *client;
> +
> +        client = new0(DHCPClient, 1);
> +        if (!client)
> +                return NULL;
> +
> +        client->state = DHCP_STATE_INIT;
> +        client->index = -1;
> +
> +        client->req_opts_size = ELEMENTSOF(default_req_opts);
> +
> +        client->req_opts = memdup(default_req_opts,
> client->req_opts_size);

Missing OOM check?

> +
> +        return client;
> +}
> diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h
> new file mode 100644
> index 0000000..65caf59
> --- /dev/null
> +++ b/src/systemd/sd-dhcp-client.h
> @@ -0,0 +1,33 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +#pragma once

If this is supposed to be public API, then it should not use the "pragma
once" stuff. That's a gcc'ism, which is fine for internal code, but for
external code we try hard to stay with C89. Please use #ifdef checks for
the public headers hence.

> +#include <netinet/in.h>
> +
> +typedef struct DHCPClient DHCPClient;

This also needs prefixing. The other public structures like this were
usually called in the style "sd_dhcp_client".

> +int sd_dhcp_client_set_request_option(DHCPClient *client, const
> uint8_t option);

Hmm, what's a "const uint8_t" parameter supposed to be? const only
really makes sense with pointer parameters, but this ins't one...

> +int sd_dhcp_client_set_request_address(DHCPClient *client,
> +                                       const struct in_addr
> *last_address);

struct in_addr? Didn't you plan to use simple uint32_t for this?

Also, if this is supposed to cover ipv6 one day too, maybe call this
sd_dhcp_client_set_request_address4() or so?

> +int sd_dhcp_client_set_index(DHCPClient *client, const int interface_index);
> +
> +DHCPClient *sd_dhcp_client_new(void);

Hmm, for the public APIs we prefer returning newly allocated objects via
a call-by-ref pointer, and return an error code as return code. i.e.:

int sd_dhcp_client_new(sd_dhcp_client *ret);

or something similar. This leaves more room open to indicate more than
just OOM errors to the caller.

Lennart

-- 
Lennart Poettering, Red Hat