From 362dcfc5db0271cd6b3a564c528cabf0ac0e7993 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 5 Apr 2024 13:40:11 +0200
Subject: [PATCH 1/2] sd-notify: fix unsetting of environment variable on error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The man page says that the variable will be unset even if the call fails.
But we weren't consistent: for some errors we would return early. This
is not good, because based on the man page, the caller may use just print
a warning, or even use '(void) sd_notify(…)' for the call.
---
 src/libsystemd/sd-daemon/sd-daemon.c | 40 ++++++++++++++++++----------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c
index 0eb0797e08f..2ff9537a6d9 100644
--- a/src/libsystemd/sd-daemon/sd-daemon.c
+++ b/src/libsystemd/sd-daemon/sd-daemon.c
@@ -650,7 +650,7 @@ _public_ int sd_notify(int unset_environment, const char *state) {
 
 _public_ int sd_pid_notifyf(pid_t pid, int unset_environment, const char *format, ...) {
         _cleanup_free_ char *p = NULL;
-        int r;
+        int r = 0, k;
 
         if (format) {
                 va_list ap;
@@ -659,16 +659,20 @@ _public_ int sd_pid_notifyf(pid_t pid, int unset_environment, const char *format
                 r = vasprintf(&p, format, ap);
                 va_end(ap);
 
-                if (r < 0 || !p)
-                        return -ENOMEM;
+                if (r < 0 || !p) {
+                        r = -ENOMEM;
+                        p = mfree(p);  /* If vasprintf failed, do not use the string,
+                                        * even if something was returned. */
+                }
         }
 
-        return sd_pid_notify(pid, unset_environment, p);
+        k = sd_pid_notify(pid, unset_environment, p);
+        return r < 0 ? r : k;
 }
 
 _public_ int sd_notifyf(int unset_environment, const char *format, ...) {
         _cleanup_free_ char *p = NULL;
-        int r;
+        int r = 0, k;
 
         if (format) {
                 va_list ap;
@@ -677,11 +681,15 @@ _public_ int sd_notifyf(int unset_environment, const char *format, ...) {
                 r = vasprintf(&p, format, ap);
                 va_end(ap);
 
-                if (r < 0 || !p)
-                        return -ENOMEM;
+                if (r < 0 || !p) {
+                        r = -ENOMEM;
+                        p = mfree(p);  /* If vasprintf failed, do not use the string,
+                                        * even if something was returned. */
+                }
         }
 
-        return sd_pid_notify(0, unset_environment, p);
+        k = sd_pid_notify(0, unset_environment, p);
+        return r < 0 ? r : k;
 }
 
 _public_ int sd_pid_notifyf_with_fds(
@@ -691,27 +699,31 @@ _public_ int sd_pid_notifyf_with_fds(
                 const char *format, ...) {
 
         _cleanup_free_ char *p = NULL;
-        int r;
+        int r = 0, k;
 
         /* Paranoia check: we traditionally used 'unsigned' as array size, but we nowadays more correctly use
          * 'size_t'. sd_pid_notifyf_with_fds() and sd_pid_notify_with_fds() are from different eras, hence
          * differ in this. Let's catch resulting incompatibilites early, even though they are pretty much
          * theoretic only */
         if (n_fds > UINT_MAX)
-                return -E2BIG;
+                r = -E2BIG;
 
-        if (format) {
+        else if (format) {
                 va_list ap;
 
                 va_start(ap, format);
                 r = vasprintf(&p, format, ap);
                 va_end(ap);
 
-                if (r < 0 || !p)
-                        return -ENOMEM;
+                if (r < 0 || !p) {
+                        r = -ENOMEM;
+                        p = mfree(p);  /* If vasprintf failed, do not use the string,
+                                        * even if something was returned. */
+                }
         }
 
-        return sd_pid_notify_with_fds(pid, unset_environment, p, fds, n_fds);
+        k = sd_pid_notify_with_fds(pid, unset_environment, p, fds, n_fds);
+        return r < 0 ? r : k;
 }
 
 _public_ int sd_booted(void) {

From f1a090b13648b19e31b03f10973be176b5b16d14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 5 Apr 2024 13:43:10 +0200
Subject: [PATCH 2/2] man: sd_notify() does not fail if var is unset

---
 man/sd_notify.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man/sd_notify.xml b/man/sd_notify.xml
index 22b351cad31..181b3a2298a 100644
--- a/man/sd_notify.xml
+++ b/man/sd_notify.xml
@@ -104,8 +104,8 @@
     <para>If the <parameter>unset_environment</parameter> parameter is non-zero,
     <function>sd_notify()</function> will unset the <varname>$NOTIFY_SOCKET</varname> environment variable
     before returning (regardless of whether the function call itself succeeded or not). Further calls to
-    <function>sd_notify()</function> will then fail, and the variable is no longer inherited by child
-    processes.</para>
+    <function>sd_notify()</function> will then silently do nothing, and the variable is no longer inherited
+    by child processes.</para>
 
     <para>The <parameter>state</parameter> parameter should contain a newline-separated list of variable
     assignments, similar in style to an environment block. A trailing newline is implied if none is