From ff05573f0ddbf7f5cf9f3c81b9c183ff05494ab1 Mon Sep 17 00:00:00 2001
From: Craig Small <csmall@enc.com.au>
Date: Sat, 4 Feb 2017 11:44:01 +1100
Subject: [PATCH] pkill: Return 0 if successfully killed process
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Original report:
When trying kill a process with insufficient privileges (see blow),
pkill displays the error message “... failed: Operation not permitted”,
but returns 0. Surely it should return 3?

$ pkill syslogd ; echo $?
pkill: killing pid 373 failed: Operation not permitted
0

Return value 0 means one of more things matched. For a pgrep (which
shares code with pkill) this makes sense, there was a match. It seems
wrong for pkill to return 0 when it in fact could not do what you told
it to.  However return value 3 means a fatal error and it's not fatal.

Looking at other programs when trying to kill things it cannot kill.
shell kill returns 1, procps kill returns 1, killall returns 1, skill
returns 0 (and says it was successful!, ah well poor old skill)

The consensus seems to be that you return 1 if you cannot kill it, even
if you found it. In other words the return value for both not found and
not able to kill it is the same.

pkill only returns 0 if something was killed. This means we found a
match AND the kill() system call worked too.

References:
 https://bugs.debian.org/852758

Signed-off-by: Craig Small <csmall@enc.com.au>
---
 NEWS    |  1 +
 pgrep.1 | 18 ++++++++++++------
 pgrep.c |  3 +++
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 2985614b..e7597d2e 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,7 @@ procps-ng-NEXT
   * library: don't strip off wchan prefixes (ps & top)     Redhat #1322111
   * top: add config file support for XDG specification
   * pgrep: warn about 15+ char name only if -f not used
+  * pkill: Return 0 only if we can kill process            Debian #852758
 
 procps-ng-3.3.12
 ----------------
diff --git a/pgrep.1 b/pgrep.1
index 8578965f..f2aefff2 100644
--- a/pgrep.1
+++ b/pgrep.1
@@ -1,8 +1,13 @@
-.\" Manual page for pgrep / pkill.
-.\" Licensed under version 2 of the GNU General Public License.
-.\" Copyright 2000 Kjetil Torgrim Homme
 .\"
-.TH PGREP "1" "September 2015" "procps-ng" "User Commands"
+.\" Copyright 2000 Kjetil Torgrim Homme
+.\"           2017 Craig Small
+.\"
+.\" This program is free software; you can redistribute it and/or modify
+.\" it under the terms of the GNU General Public License as published by
+.\" the Free Software Foundation; either version 2 of the License, or
+.\" (at your option) any later version.
+.\"
+.TH PGREP "1" "2017-02-04" "procps-ng" "User Commands"
 .SH NAME
 pgrep, pkill \- look up or signal processes based on name and other attributes
 .SH SYNOPSIS
@@ -197,10 +202,11 @@ $ renice +4 $(pgrep netscape)
 .PD 0
 .TP
 0
-One or more processes matched the criteria.
+One or more processes matched the criteria. For pkill the process must also
+have been successfully signalled.
 .TP
 1
-No processes matched.
+No processes matched or none of them could be signalled.
 .TP
 2
 Syntax error in the command line.
diff --git a/pgrep.c b/pgrep.c
index 82f7d6e8..7aae2584 100644
--- a/pgrep.c
+++ b/pgrep.c
@@ -866,10 +866,12 @@ int main (int argc, char **argv)
     procs = select_procs (&num);
     if (i_am_pkill) {
         int i;
+        int kill_count = 0;
         for (i = 0; i < num; i++) {
             if (kill (procs[i].num, opt_signal) != -1) {
                 if (opt_echo)
                     printf(_("%s killed (pid %lu)\n"), procs[i].str, procs[i].num);
+                kill_count++;
                 continue;
             }
             if (errno==ESRCH)
@@ -879,6 +881,7 @@ int main (int argc, char **argv)
         }
         if (opt_count)
             fprintf(stdout, "%d\n", num);
+        return !kill_count;
     } else {
         if (opt_count) {
             fprintf(stdout, "%d\n", num);