sd-notify: fix unsetting of environment variable on error

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.

1 files changed, 26 insertions, 14 deletions

diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c
index 0eb0797e08..2ff9537a6d 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) {