[systemd-devel] [PATCH 11/28] dhcp: Add function for sending a raw packet

Lennart Poettering lennart at poettering.net
Wed Nov 13 16:15:36 PST 2013


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

> +int __dhcp_network_send_raw_packet(int index, void *packet, int len);

Shouldn't this be "const void *"?

>  int __dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,

buflen should better be size_t, no?

>                           uint8_t optlen, void *optval);

optval again const?

> +int __dhcp_network_send_raw_packet(int index, void *packet, int len)
> +{
> +        int err, s;
> +        struct sockaddr_ll link;
> +
> +        err = 0;
> +
> +        s = socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC, htons(ETH_P_IP));
> +        if (s < 0)
> +                return -errno;
> +
> +        memset(&link, 0, sizeof(link));

Please initialize the thing when you define it:

   struct sockaddr_ll link = {}; 

It's actually quicker. Ask Zbigniew... ;-)

In cases where this kind of initialization doesn't work, use the zero()
macro for ases like this.

> +
> +        link.sll_family = AF_PACKET;
> +        link.sll_protocol = htons(ETH_P_IP);
> +        link.sll_ifindex =  index;
> +        link.sll_halen = ETH_ALEN;
> +        memset(&link.sll_addr, 0xff, ETH_ALEN);
> +
> +        if (bind(s, (struct sockaddr *)&link, sizeof(link)) < 0) {

Hmm, to never even have to think about aliasing issue we generally try
to use unions for cases like this. In fact in "socket-util.h" you find a
definition of "union sockaddr_union", where you could add
sockaddr_ll. If you then use the union, you can avoid the cast easily by
getting a pointer of the "sa" member of it.

> +                err = -errno;
> +                close(s);
> +                return err;
> +        }

> +
> +        if (sendto(s, packet, len, 0, (struct sockaddr *)&link,
> +                   sizeof(link)) < 0)
> +                err = -errno;
> +
> +        close(s);
> +
> +        return err;
> +}

Sounds like you want to use _cleanup_close_ here!

Lennart

-- 
Lennart Poettering, Red Hat


More information about the systemd-devel mailing list