From 362dcfc5db0271cd6b3a564c528cabf0ac0e7993 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 5 Apr 2024 13:40:11 +0200 Subject: 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 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) { -- cgit v1.2.3