Hi, On Tue, 2013-11-26 at 00:23 +0100, Lennart Poettering wrote: > On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote: > > > +static int client_receive_raw_message(sd_event_source *s, int fd, > > + uint32_t revents, void *userdata) > > +{ > > + DHCPClient *client = userdata; > > + int len, buflen; > > + uint8_t *buf; > > + uint8_t tmp; > > + DHCPPacket *message; > > + > > + 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? > > + > > + len = read(fd, buf, buflen); > > + if (len < 0) > > + 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 = -1; > > + client->receive_message = > > + > > sd_event_source_unref(client->receive_message); > > > You should unref the source before closing the fd, as this internally > still references the fd. Ok. > > -int dhcp_network_send_raw_packet(int index, void *packet, int len) > > +int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link, > > + void *packet, int len) > > Hmm, what's the story regarding blocking here? What happens if the > socket is full? This call will block then, which sucks. Also, if you are > not using SOCK_NONBLOCK on these sockets, epoll behaviour can be weird, > as the socket layer takes the freedom to wake you up sometimes without > any packet to read. (We had the issue in Avahi). > > It sounds to me as if you should set SOCK_NONBLOCK and then set a socket > send buffer (SO_SNDBUF) as large as useful, and simply consider it a > real error if you the ever get EAGAIN on sending... I'll fix... Patrik _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel