Re: [pacman-dev] [PATCH] pacman: Correct signal handler behavior

20 Jun 2014

On Fri, Jun 20, 2014 at 05:12:10PM -0400, Andrew Gregory wrote:
...
On 06/20/14 at 10:00pm, Silvan Jegen wrote:
...
One of the comments for this function is out of sync with the code.
If a SIGHUP is received the code calls the release function only if
there is no transaction in flight (contrary to the comments).
This change brings the code in line with the comment (and moves the
comment for readability). Additionally, it changes the exit message
to reflect the fact that the signal received was either a SIGHUP or
a SIGTERM.
Signed-off-by: Silvan Jegen 
---
 src/pacman/pacman.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
I think the comment is just a typo.  According to its commit message,
SIGHUP and SIGINT should be treated the same.  SIGHUP seems like
a poor reason to kill an active transaction rather than more
gracefully interrupt it.
I tend to agree.

Should I send a patch to change the comment accordingly (and maybe move
it before the "else if(signum == SIGINT || signum == SIGHUP) {" in the
original code)?
...
apg
...

diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index ef86d39..599a141 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -301,20 +301,20 @@ static void handler(int signum)
      	"Please submit a full bug report with --debug if appropriate.\n";
      xwrite(err, msg, strlen(msg));
      exit(signum);
-	} else if(signum == SIGINT || signum == SIGHUP) {
-		if(signum == SIGINT) {
-			msg = "\nInterrupt signal received\n";
-		} else {
-			msg = "\nHangup signal received\n";
-		}
-		xwrite(err, msg, strlen(msg));
+	}
+	/* SIGINT: no committing transaction, release it now and then exit pacman
+	 * SIGHUP, SIGTERM: release no matter what */
+	if(signum == SIGINT) {
+		msg = "\nInterrupt signal received\n";
      if(alpm_trans_interrupt(config->handle) == 0) {
      	/* a transaction is being interrupted, don't exit pacman yet. */
      	return;
      }
+	} else {
+		msg = "\nHangup/Termination signal received\n";
  }
-	/* SIGINT: no committing transaction, release it now and then exit pacman
-	 * SIGHUP, SIGTERM: release no matter what */
+
+	xwrite(err, msg, strlen(msg));
  alpm_trans_release(config->handle);
  /* output a newline to be sure we clear any line we may be on */
  xwrite(out, "\n", 1);
-- 
2.0.0

    