NEWS and some fixes for exit handling

Add a NEWS entry for the change in exit status handling, fix the
k5start/sigchld test, and add a krenew test.  Fix a style issue
with a C++-style comment.

diff --git a/NEWS b/NEWS
index f6eaf36d6199e446fb0bc6059df5f50c25829d62..c4833fb86e254f5dfd395fe2fbfa6291b2142414 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,12 @@
 
 kstart 4.3 (unreleased)
 
+    If the process run by k5start or krenew is killed by a signal, k5start
+    or krenew now exits with status 128 plus the killing signal rather
+    than zero.  This avoids the caller of k5start or krenew thinking the
+    command succeeded when it was killed, and matches the return status
+    behavior of bash.  Patch from Aasif Versi.
+
     Rename the script to bootstrap from a Git checkout to bootstrap,
     matching the emerging consensus in the Autoconf world.
 

diff --git a/tests/k5start/basic-t b/tests/k5start/basic-t
index 3eb608b435c001a800938aa7faec29efdf8494fc..545336c3cf49a643882bedb12c51f53ba819fbcd 100755 (executable)
--- a/tests/k5start/basic-t
+++ b/tests/k5start/basic-t
@@ -9,6 +9,7 @@
 #
 # SPDX-License-Identifier: MIT
 
+use POSIX qw(SIGTERM);
 use Test::More;
 
 # The full path to the newly-built k5start client.
@@ -294,7 +295,7 @@ unlink 'krb5cc_test';
 ($out, $err, $status)
     = command ($K5START, '-Uqf', "$DATA/test.keytab", '--', 'sh', '-c',
                'kill $$');
-is ($status, 143, 'k5start of kill $$ returns correct exit status');
+is ($status, 128 + SIGTERM, 'k5start of kill $$ returns correct exit status');
 is ($err, '', ' with no errors');
 ok (!-f 'krb5cc_test', ' and the default cache file was not created');
 

diff --git a/tests/k5start/sigchld-t b/tests/k5start/sigchld-t
index 06e9740e52755df905efa635bfe0324e8181bdda..02be6a9dc7e3e1176fd13d5b584cffb271a293a8 100755 (executable)
--- a/tests/k5start/sigchld-t
+++ b/tests/k5start/sigchld-t
@@ -65,6 +65,6 @@ kill (SIGCONT, $child) or BAIL_OUT ("cannot send SIGCONT to child $child\n");
 sleep 1;
 kill (SIGTERM, $child) or BAIL_OUT ("cannot send SIGTERM to child $child\n");
 waitpid ($pid, 0);
-is ($? >> 8, 0, 'command succeeded');
+is ($? >> 8, 128 + SIGTERM, 'command killed with SIGTERM');
 ok (time < $start + 5, 'k5start got SIGCHLD and woke up properly');
 unlink 'child-pid';

diff --git a/tests/krenew/basic-t b/tests/krenew/basic-t
index 49320fe61d8f213d67e944d03eb50eabc7a6292f..c66f005c778174c458bc1b859e6b165a438bc208 100755 (executable)
--- a/tests/krenew/basic-t
+++ b/tests/krenew/basic-t
@@ -10,6 +10,7 @@
 # SPDX-License-Identifier: MIT
 
 use IPC::Open3 qw(open3);
+use POSIX qw(SIGTERM);
 use Test::More;
 
 # The full path to the newly-built krenew client.
@@ -34,7 +35,7 @@ if (not -f "$DATA/test.keytab" or not -f "$DATA/test.principal") {
         plan skip_all => 'cannot get renewable tickets';
         exit 0;
     }
-    plan tests => 36;
+    plan tests => 39;
 }
 
 # We should be okay for five minute tickets without doing anything.
@@ -99,6 +100,12 @@ like ($out, qr,^\s*(Ticket|Credentials)\ cache:
 ok (!$cache || !-f $cache, ' and the new cache file was deleted');
 ok (-f 'krb5cc_test', ' and the default cache file was unaffected');
 
+# Test propagation of exit status from a command which is killed by signal.
+($out, $err, $status) = command ($KRENEW, '--', 'sh', '-c', 'kill $$');
+is ($status, 128 + SIGTERM, 'krenew of kill $$ returns correct exit status');
+is ($err, '', ' with no errors');
+ok (-f 'krb5cc_test', ' and the default cache file was unaffected');
+
 # Check exit status if the ticket cache doesn't exist.
 unlink 'krb5cc_test';
 ($out, $err, $status) = command ($KRENEW);

diff --git a/util/command.c b/util/command.c
index 8fccab7662938cfc035f4585a8caf8cd9dab9b80..45501b8b94577b84dcdf974fcadadc759be8f829 100644 (file)
--- a/util/command.c
+++ b/util/command.c
@@ -135,7 +135,10 @@ command_finish(pid_t child, int *status)
         if (WIFEXITED(*status))
             *status = WEXITSTATUS(*status);
         else if (WIFSIGNALED(*status))
-            // +128 to match exit status set by bash when process is signaled
+            /*
+             * Use the adjusted process signal as the exit status.  This
+             * duplicates the exit status behavior of bash.
+             */
             *status = WTERMSIG(*status) + 128;
     }