[systemd-devel] [systemd-commits] 2 commits - src/libsystemd-network src/udev

[Lennart Poettering]

On Sat, 14.03.15 06:54, Ronny Chevalier (rchevalier at kemper.freedesktop.org) wrote:

> commit 6ec8e7c763b7dfa82e25e31f6938122748d1608f
> Author: Shawn Landden <shawn at churchofgit.com>
> Date:   Tue Mar 10 20:45:15 2015 -0700
> 
>     sd-dhcp-client: fix strict aliasing issue
> 
> diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
> index 4224e01..a477ccc 100644
> --- a/src/libsystemd-network/sd-dhcp-client.c
> +++ b/src/libsystemd-network/sd-dhcp-client.c
> @@ -1469,7 +1469,7 @@ static int client_receive_message_udp(sd_event_source *s, int fd,
>          _cleanup_free_ DHCPMessage *message = NULL;
>          int buflen = 0, len, r;
>          const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
> -        const struct ether_addr *expected_chaddr = NULL;
> +        bool expect_chaddr;
>          uint8_t expected_hlen = 0;
>  
>          assert(s);
> @@ -1514,11 +1514,11 @@ static int client_receive_message_udp(sd_event_source *s, int fd,
>  
>          if (client->arp_type == ARPHRD_ETHER) {
>                  expected_hlen = ETH_ALEN;
> -                expected_chaddr = (const struct ether_addr *) &client->mac_addr;
> +                expect_chaddr = true;
>          } else {
>                 /* Non-ethernet links expect zero chaddr */
>                 expected_hlen = 0;
> -               expected_chaddr = &zero_mac;
> +               expect_chaddr = false;
>          }
>  
>          if (message->hlen != expected_hlen) {
> @@ -1526,7 +1526,10 @@ static int client_receive_message_udp(sd_event_source *s, int fd,
>                  return 0;
>          }
>  
> -        if (memcmp(&message->chaddr[0], expected_chaddr, ETH_ALEN)) {
> +        if (memcmp(&message->chaddr[0], expect_chaddr ?
> +                                          (void *)&client->mac_addr :
> +                                          (void *)&zero_mac,
> +                                        ETH_ALEN)) {
>                  log_dhcp_client(client, "received chaddr does not match "
>                                  "expected: ignoring");
>                  return 0;

Ahum, what is this about? Please elaborate what kind of struct aliasing
this fixes?

The compiler knows very well what we are taking the pointer of... and
memcmp() doesn't really care anyway it takes (void*) pointers anyway...

What's this supposed to fix? It makes something that was typesafe
before not typesafe. And it It also adds a ternary operator (which is
hardly the prettiest thing to look at), in addition to an if block,
where the if block was sufficient before.

Also, casting to (void*) is implicit, this already looks suspicious...

Lennart

-- 
Lennart Poettering, Red Hat