Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd-network
On Tue, 14.04.15 09:33, Thomas H.P. Andersen (pho...@kemper.freedesktop.org) wrote: > src/libsystemd-network/sd-dhcp6-client.c |2 ++ > src/libsystemd-network/test-dhcp6-client.c |2 -- > 2 files changed, 2 insertions(+), 2 deletions(-) > > New commits: > commit 70c79983e1abae17c46969b024d0b9e6a3b83d00 > Author: Thomas Hindoe Paaboel Andersen > Date: Tue Apr 14 18:24:00 2015 +0200 > > test-dhcp6-client: don't unref the event twice > > diff --git a/src/libsystemd-network/test-dhcp6-client.c > b/src/libsystemd-network/test-dhcp6-client.c > index 9386f31..7618547 100644 > --- a/src/libsystemd-network/test-dhcp6-client.c > +++ b/src/libsystemd-network/test-dhcp6-client.c > @@ -701,7 +701,5 @@ int main(int argc, char *argv[]) { > test_advertise_option(e); > test_client_solicit(e); > > -assert_se(!sd_event_unref(e)); > - > return 0; > } > > commit 8283c71b7141afc6ad69dc7913311aa01e8221dd > Author: Thomas Hindoe Paaboel Andersen > Date: Tue Apr 14 18:02:15 2015 +0200 > > sd-dhcp6-client: unref lease when freeing the client > > diff --git a/src/libsystemd-network/sd-dhcp6-client.c > b/src/libsystemd-network/sd-dhcp6-client.c > index 9d88d46..cd33237 100644 > --- a/src/libsystemd-network/sd-dhcp6-client.c > +++ b/src/libsystemd-network/sd-dhcp6-client.c > @@ -1205,6 +1205,8 @@ sd_dhcp6_client *sd_dhcp6_client_unref(sd_dhcp6_client > *client) { > client_reset(client); > > sd_dhcp6_client_detach_event(client); > +if (client->lease) > +sd_dhcp6_lease_unref(client->lease); A quick note: our destructor functions should all accept NULL as parameter and then become NOPs. sd_dhcp6_lease_unref() does that correctly, and hence makes the explicit if check by the caller unnecessary. Will fix. Will also add a note to CODING_STYLE about this. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd
On Tue, Apr 14, 2015 at 4:21 PM, Zbigniew Jędrzejewski-Szmek wrote: > On Tue, Apr 14, 2015 at 07:19:42AM -0700, Tom Gundersen wrote: >> src/libsystemd/sd-device/sd-device.c |9 ++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> New commits: >> commit 85091685af65831f379580c75b40776c20e245ee >> Author: Tom Gundersen >> Date: Tue Apr 14 16:05:53 2015 +0200 >> >> sd-device: fix reading of subsystem >> >> diff --git a/src/libsystemd/sd-device/sd-device.c >> b/src/libsystemd/sd-device/sd-device.c >> index 7d52e3c..d420bd0 100644 >> --- a/src/libsystemd/sd-device/sd-device.c >> +++ b/src/libsystemd/sd-device/sd-device.c >> @@ -772,10 +772,10 @@ _public_ int sd_device_get_subsystem(sd_device >> *device, const char **ret) { >> r = device_set_subsystem(device, "drivers"); >> else if (path_startswith(device->devpath, "/subsystem/") || >> path_startswith(device->devpath, "/class/") || >> - path_startswith(device->devpath, "/buss/")) >> + path_startswith(device->devpath, "/bus/")) >> r = device_set_subsystem(device, "subsystem"); >> if (r < 0) >> -return r; >> +return log_debug_errno(r, "sd-devcie: could not set >> subsystem for %s: %m", device->devpath); > ^^ > > Zbyszek Thanks! Fixed. -t ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd
On Tue, Apr 14, 2015 at 07:19:42AM -0700, Tom Gundersen wrote: > src/libsystemd/sd-device/sd-device.c |9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > New commits: > commit 85091685af65831f379580c75b40776c20e245ee > Author: Tom Gundersen > Date: Tue Apr 14 16:05:53 2015 +0200 > > sd-device: fix reading of subsystem > > diff --git a/src/libsystemd/sd-device/sd-device.c > b/src/libsystemd/sd-device/sd-device.c > index 7d52e3c..d420bd0 100644 > --- a/src/libsystemd/sd-device/sd-device.c > +++ b/src/libsystemd/sd-device/sd-device.c > @@ -772,10 +772,10 @@ _public_ int sd_device_get_subsystem(sd_device *device, > const char **ret) { > r = device_set_subsystem(device, "drivers"); > else if (path_startswith(device->devpath, "/subsystem/") || > path_startswith(device->devpath, "/class/") || > - path_startswith(device->devpath, "/buss/")) > + path_startswith(device->devpath, "/bus/")) > r = device_set_subsystem(device, "subsystem"); > if (r < 0) > -return r; > +return log_debug_errno(r, "sd-devcie: could not set > subsystem for %s: %m", device->devpath); ^^ Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd-network src/udev
On Sun, Apr 12, 2015 at 1:49 AM, Shawn Landden wrote: > Given that we don't do any operations on it (besides memcmp which doesn't > matter) > it doesn't actually violate, but it does generate an annoying warning. What about making expected_chaddr a void pointer? Would that remove the warning? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd-network src/udev
On Fri, Apr 10, 2015 at 07:54:33PM +0200, Ronny Chevalier wrote: > On Fri, Apr 10, 2015 at 7:05 PM, Lennart Poettering > wrote: > > On Sat, 14.03.15 06:54, Ronny Chevalier (rcheval...@kemper.freedesktop.org) > > wrote: > > > >> commit 6ec8e7c763b7dfa82e25e31f6938122748d1608f > >> Author: Shawn Landden > >> 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? > > I think Shawn was trying to avoid to cast mac_addr which is (uint8_t > *) to a (const struct ether_addr *) which breaks the strict aliasing > rule. Or I misunderstood the patch? Shawn? > >> -expected_chaddr = (const struct ether_addr *) > >> &client->mac_addr; > > > > > > The compiler knows very well what we are taking the pointer of... and > > memcmp() doesn't really care anyway it takes (void*) pointers anyway... > > The issue is not with memcmp but with the cast from a (uint8_t *) to a > (const struct ether_addr *) which breaks the strict aliasing rule. Given that we don't do any operations on it (besides memcmp which doesn't matter) it doesn't actually violate, but it does generate an annoying warning. > > If I'm not mistaken Shawn is trying to remove all the strict aliasing > violations reported by gcc, in order to add the warning later on. > > Ronny ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd
On Sun, Apr 5, 2015 at 11:50 AM, Lennart Poettering wrote: > On Fri, 03.04.15 13:23, Tom Gundersen (tome...@kemper.freedesktop.org) wrote: > >> >> -tags = strdupa(value); >> +FOREACH_WORD_SEPARATOR(word, l, value, ":", state) { >> +char *tag; >> >> -while ((next = strchr(tags, ':'))) { >> -next[0] = '\0'; >> +tag = strndupa(word, l); >> > > Repeat after me: I shall not use alloca() within loops. I shall not > use alloca() within loops. I shall not use alloca() within loops. > > This cannot work. Fixed. Thanks! Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd
On Fri, 03.04.15 13:23, Tom Gundersen (tome...@kemper.freedesktop.org) wrote: > > -tags = strdupa(value); > +FOREACH_WORD_SEPARATOR(word, l, value, ":", state) { > +char *tag; > > -while ((next = strchr(tags, ':'))) { > -next[0] = '\0'; > +tag = strndupa(word, l); > Repeat after me: I shall not use alloca() within loops. I shall not use alloca() within loops. I shall not use alloca() within loops. This cannot work. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd src/network
On Sat, Mar 8, 2014 at 6:23 PM, Zbigniew Jędrzejewski-Szmek wrote: > On Fri, Mar 07, 2014 at 04:13:06PM -0800, Tom Gundersen wrote: >> src/libsystemd/sd-rtnl/rtnl-internal.h |2 +- >> src/network/networkd-link.c|6 ++ >> 2 files changed, 3 insertions(+), 5 deletions(-) >> >> New commits: >> commit 76800848f281c3705c9364fd3e888153d94aaf34 >> Author: Tom Gundersen >> Date: Sat Mar 8 01:08:30 2014 +0100 >> >> networkd: link - degrade failed UP to warning >> >> Something else may still bring the link up, so don't enter failed state >> prematurely. >> >> diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c >> index 785e8d5..9fb8d9f 100644 >> --- a/src/network/networkd-link.c >> +++ b/src/network/networkd-link.c >> @@ -1056,15 +1056,13 @@ static int link_up_handler(sd_rtnl *rtnl, >> sd_rtnl_message *m, void *userdata) { >> return 1; >> >> r = sd_rtnl_message_get_errno(m); >> -if (r < 0) { >> -log_struct_link(LOG_ERR, link, >> +if (r < 0) >> +log_struct_link(LOG_WARNING, link, >> "MESSAGE=%s: could not bring up interface: >> %s", >> link->ifname, strerror(-r), >> "ERRNO=%d", -r, >> NULL); >> -link_enter_failed(link); >> return 1; >> -} > Hi Tom, > > link_update_flags is unreachable. I pushed an update, please check > if it looks ok. Looks good. Thanks! Cheers, Tom ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 2 commits - src/libsystemd src/network
On Fri, Mar 07, 2014 at 04:13:06PM -0800, Tom Gundersen wrote: > src/libsystemd/sd-rtnl/rtnl-internal.h |2 +- > src/network/networkd-link.c|6 ++ > 2 files changed, 3 insertions(+), 5 deletions(-) > > New commits: > commit 76800848f281c3705c9364fd3e888153d94aaf34 > Author: Tom Gundersen > Date: Sat Mar 8 01:08:30 2014 +0100 > > networkd: link - degrade failed UP to warning > > Something else may still bring the link up, so don't enter failed state > prematurely. > > diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c > index 785e8d5..9fb8d9f 100644 > --- a/src/network/networkd-link.c > +++ b/src/network/networkd-link.c > @@ -1056,15 +1056,13 @@ static int link_up_handler(sd_rtnl *rtnl, > sd_rtnl_message *m, void *userdata) { > return 1; > > r = sd_rtnl_message_get_errno(m); > -if (r < 0) { > -log_struct_link(LOG_ERR, link, > +if (r < 0) > +log_struct_link(LOG_WARNING, link, > "MESSAGE=%s: could not bring up interface: > %s", > link->ifname, strerror(-r), > "ERRNO=%d", -r, > NULL); > -link_enter_failed(link); > return 1; > -} Hi Tom, link_update_flags is unreachable. I pushed an update, please check if it looks ok. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel