all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] applied: [PATCH 1/2] run command: avoid using 1 as special value
Date: Mon, 26 Feb 2024 15:10:19 +0100	[thread overview]
Message-ID: <20240226141020.296052-1-t.lamprecht@proxmox.com> (raw)

In Perl, the last expression of a block (e.g. of a method, eval) gets
returned if there's no explicit return statement. Quite often that is
truthy, i.e., 1.

As that was chosen as the special value for the CMD_FINISHED flag it
had quite a few false positives, causing weird effects and
installation failure.

Reserve that overly problematic value and chose 2 as new CMD_FINISHED
value, albeit it could be better to signal this even more explicitly,
like with a structured hash reference, but for now this is a good stop
gap.

Fixes: 23c5fbe ("sys: command: allow terminating the process early from log subroutine")
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 Proxmox/Sys/Command.pm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Proxmox/Sys/Command.pm b/Proxmox/Sys/Command.pm
index cb5fe76..ac505c4 100644
--- a/Proxmox/Sys/Command.pm
+++ b/Proxmox/Sys/Command.pm
@@ -17,7 +17,10 @@ use Proxmox::UI;
 use base qw(Exporter);
 our @EXPORT_OK = qw(run_command syscmd CMD_FINISHED);
 
-use constant CMD_FINISHED => 1;
+use constant {
+    CMD_RESERVED => 1<<0, # reserve 1 as it's often the default return value of closures
+    CMD_FINISHED => 1<<1,
+};
 
 my sub shellquote {
     my $str = shift;
@@ -78,8 +81,9 @@ sub syscmd {
 #
 # Arguments:
 # * $cmd - The command to run, either a single string or array with individual arguments
-# * $func - Logging subroutine to call, receives both stdout and stderr. Might return CMD_FINISHED
-#           to exit early and ignore the rest of the process output.
+# * $func - Logging subroutine to call, receives both stdout and stderr.
+#           Can return CMD_FINISHED to exit early and ignore the rest of the process output.
+#           Should use an explicit  `return;` to avoid misinterpretation of return value.
 # * $input - Stdin contents for the spawned subprocess
 # * $noout - Whether to append any process output to the return value
 # * $noprint - Whether to print any process output to the parents stdout
-- 
2.39.2





             reply	other threads:[~2024-02-26 14:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-26 14:10 Thomas Lamprecht [this message]
2024-02-26 14:10 ` [pve-devel] applied: [PATCH 2/2] run command: use explicit return undef in closures on call sites Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240226141020.296052-1-t.lamprecht@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal