On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel