Skip to contentSkip to conversation

systemd: imported fix for journald SIGABRTs.

PATCH New Jordan Yelloz 3 participants 5 messages Oct 13, 2015 – Oct 14, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 systemd: imported fix for journald SIGABRTs.

Prior to applying this patch, assertion errors would reliably be displayed on the console. These failed assertions led to crashes in systemd-journald on startup.

This patch fixes systemd bug #512:
<https://github.com/systemd/systemd/issues/512>

---
 .../0004-sd-device-never-return-NULL-0.patch       | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 package/systemd/0004-sd-device-never-return-NULL-0.patch
package/systemd/0004-sd-device-never-return-NULL-0.patch new file +55 -0
new file mode 100644
index 0000000..a0aa543
@@ -0,0 +1,55 @@
+From aad85277b519c700c33a39f2e6558384d2d4aa9f Mon Sep 17 00:00:00 2001
+From: David Herrmann <[email protected]>
+Date: Wed, 15 Jul 2015 12:30:08 +0200
+Subject: [PATCH 1/1] sd-device: never return NULL+0
+
+It is highly confusing if a getter function returns 0, but the value is
+set to NULL. This, right now, triggers assertions as code relies on the
+returned values to be non-NULL.
+
+Like with sd-bus-creds and friends, return 0 only if a value is actually
+available.
+
+Discussed with Tom, and actually fixes real bugs as in #512.
+
+Signed-off-by: Jordan Yelloz <[email protected]>
+---
+ src/libsystemd/sd-device/sd-device.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+
+diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
+index b274f71..7cea5a0 100644
+--- a/src/libsystemd/sd-device/sd-device.c
++++ b/src/libsystemd/sd-device/sd-device.c
+@@ -791,6 +791,9 @@ _public_ int sd_device_get_subsystem(sd_device *device, const char **ret) {
+                 device->subsystem_set = true;
+         }
+ 
++        if (!device->subsystem)
++                return -ENOENT;
++
+         *ret = device->subsystem;
+ 
+         return 0;
+@@ -908,6 +911,9 @@ _public_ int sd_device_get_driver(sd_device *device, const char **ret) {
+                         return log_debug_errno(r, "sd-device: could not set driver for %s: %m", device->devpath);
+         }
+ 
++        if (!device->driver)
++                return -ENOENT;
++
+         *ret = device->driver;
+ 
+         return 0;
+@@ -1002,6 +1008,8 @@ _public_ int sd_device_get_sysname(sd_device *device, const char **ret) {
+                         return r;
+         }
+ 
++        assert_return(device->sysname, -ENOENT);
++
+         *ret = device->sysname;
+ 
+         return 0;
+-- 
+2.6.1
+
Signature · 1 line
2.6.1
#1 lore raw

PATCH 1/1 systemd: imported fix for journald SIGABRTs.

Hello all,

Quoted 8 lines · Jordan Yelloz

Prior to applying this patch, assertion errors would reliably be displayed on the
console. These failed assertions led to crashes in systemd-journald on startup.

This patch fixes systemd bug #512:
<https://github.com/systemd/systemd/issues/512>

NAK on this one. There's already a patch in the queue to bump systemd to v226 (and it should be trumped with a patch to update to v227 but it requires some others patches so it'll take more time) that includes this patch.
https://patchwork.ozlabs.org/patch/525867/

Quoted 40 lines
---
 .../0004-sd-device-never-return-NULL-0.patch       | 55
++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644

package/systemd/0004-sd-device-never-return-NULL-0.patch

package/systemd/0004-sd-device-never-return-NULL-0.patch new file +19 -0
new file mode 100644
index 0000000..a0aa543
@@ -0,0 +1,55 @@
+From aad85277b519c700c33a39f2e6558384d2d4aa9f Mon Sep 17 00:00:00 2001
+From: David Herrmann <[email protected]>
+Date: Wed, 15 Jul 2015 12:30:08 +0200
+Subject: [PATCH 1/1] sd-device: never return NULL+0
+
+It is highly confusing if a getter function returns 0, but the value is
+set to NULL. This, right now, triggers assertions as code relies on the
+returned values to be non-NULL.
+
+Like with sd-bus-creds and friends, return 0 only if a value is actually
+available.
+
+Discussed with Tom, and actually fixes real bugs as in #512.
+
+Signed-off-by: Jordan Yelloz <[email protected]>
+---
+ src/libsystemd/sd-device/sd-device.c | 8 ++++++++
+ 1 file changed, 8 insertions(+)
+

[-SNIP-]
+
--
2.6.1

Quoted 5 lines

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151013/692ade26/attachment.html>

#2 lore raw

PATCH 1/1 systemd: imported fix for journald SIGABRTs.

Maxime, Jordan,

Quoted 11 lines · Maxime Hadjinlian
Quoted 8 lines · Jordan Yelloz

Prior to applying this patch, assertion errors would reliably be displayed on the
console. These failed assertions led to crashes in systemd-journald on startup.

This patch fixes systemd bug #512:
<https://github.com/systemd/systemd/issues/512>

NAK on this one.

I clearly don't think "NAK on this one" is the appropriate way to react to a contribution, especially when the contribution in question is only the second one from Jordan.

Here is a much more appropriate response for Jordan:

"""
Jordan,

Thanks for your patch! However, it turns out that another patch was already posted some time ago to bump the systemd version from 221 to 226 (see https://patchwork.ozlabs.org/patch/525867/). Since systemd v226 already includes the fix for the problem you're pointing to, it seems like a better idea to upgrade systemd to v226 rather than backporting just this one fix.

Consequently, we will mark your patch as "Rejected" in our patch tracking system, as we will favor the systemd update instead. However, what you could do is test the patch at
https://patchwork.ozlabs.org/patch/525867/ and if it works for you, report a Tested-by.

Thanks!
"""

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH 1/1 systemd: imported fix for journald SIGABRTs.

Hi Jordan

Quoted 15 lines · Thomas Petazzoni

Maxime, Jordan,

Quoted 3 lines · Maxime Hadjinlian
Quoted 1 line · Jordan Yelloz

Prior to applying this patch, assertion errors would reliably be

displayed

Quoted 7 lines
Quoted 7 lines

on the
console. These failed assertions led to crashes in systemd-journald on startup.

This patch fixes systemd bug #512:
<https://github.com/systemd/systemd/issues/512>

Quoted 8 lines
Quoted 1 line

NAK on this one.

I clearly don't think "NAK on this one" is the appropriate way to react to a contribution, especially when the contribution in question is only the second one from Jordan.

Here is a much more appropriate response for Jordan:

I apologies about the tone of my mail, I though you were an older contributor I seems to recalled your name, anyway, that's not an excuse for this.
Don't take it bad and please send many more patches :)

Quoted 20 lines

"""
Jordan,

Thanks for your patch! However, it turns out that another patch was already posted some time ago to bump the systemd version from 221 to 226 (see https://patchwork.ozlabs.org/patch/525867/). Since systemd v226 already includes the fix for the problem you're pointing to, it seems like a better idea to upgrade systemd to v226 rather than backporting just this one fix.

Consequently, we will mark your patch as "Rejected" in our patch tracking system, as we will favor the systemd update instead. However, what you could do is test the patch at
https://patchwork.ozlabs.org/patch/525867/ and if it works for you, report a Tested-by.

Thanks!
"""

Quoted 8 lines

Best regards,

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151013/67dc5dc4/attachment.html>

#4 lore raw

PATCH 1/1 systemd: imported fix for journald SIGABRTs.

Quoted 53 lines · Maxime Hadjinlian

Hi Jordan

On Tue, Oct 13, 2015 at 9:53 AM, Thomas Petazzoni <[email protected] <mailto:[email protected]>> wrote:

Maxime, Jordan,

On Tue, 13 Oct 2015 09:47:20 +0200, Maxime Hadjinlian wrote:

> On Tue, Oct 13, 2015 at 5:58 AM, Jordan Yelloz <jordan at yelloz.me <mailto:[email protected]>> wrote:
>
> > Prior to applying this patch, assertion errors would reliably be displayed
> > on the
> > console. These failed assertions led to crashes in systemd-journald on
> > startup.
> >
> > This patch fixes systemd bug #512:
> > <https://github.com/systemd/systemd/issues/512>
> >
> NAK on this one.

I clearly don't think "NAK on this one" is the appropriate way to react to a contribution, especially when the contribution in question is only the second one from Jordan.

Here is a much more appropriate response for Jordan:

I apologies about the tone of my mail, I though you were an older contributor I seems to recalled your name, anyway, that's not an excuse for this.
Don't take it bad and please send many more patches :)

"""
Jordan,

Thanks for your patch! However, it turns out that another patch was already posted some time ago to bump the systemd version from 221 to 226 (see https://patchwork.ozlabs.org/patch/525867/). Since systemd v226 already includes the fix for the problem you're pointing to, it seems like a better idea to upgrade systemd to v226 rather than backporting just this one fix.

Consequently, we will mark your patch as "Rejected" in our patch tracking system, as we will favor the systemd update instead. However, what you could do is test the patch at
https://patchwork.ozlabs.org/patch/525867/ and if it works for you, report a Tested-by.

Thanks!
"""

Quoted 9 lines

Best regards,

Thomas
--
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering
http://free-electrons.com

It's not a big deal. I was not aware of the proposed patch to upgrade to systemd 226/227. I just assumed support for systemd was lagging behind, which is clearly not the case. I have imported patch #525867 and will test it soon.

Anyway, to provide a bit of justification to my actions, I am working off of the latest stable branch of buildroot (2015.08.x) and I saw my patch as strictly a bug-fix patch that wouldn't add any new package requirements or configuration options and one that fixes a critical defect (at least in my use case) in systemd 221.

--
Jordan