Ubuntu
psmisc package

Merge ~paelzer/ubuntu/+source/psmisc:lp1806060-fix-killall-option-parsing-cosmic into ubuntu/+source/psmisc:ubuntu/cosmic-devel

  1. Git
  2. lp:~paelzer/ubuntu/+source/psmisc
  3. lp1806060-fix-killall-option-parsing-cosmic
  4. Merge into ubuntu/cosmic-devel
Proposed by Christian Ehrhardt 
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: 0028bb2bbb245d4dc7709b8d9c531ba07ea90180
Merged at revision: 0028bb2bbb245d4dc7709b8d9c531ba07ea90180
Proposed branch: ~paelzer/ubuntu/+source/psmisc:lp1806060-fix-killall-option-parsing-cosmic
Merge into: ubuntu/+source/psmisc:ubuntu/cosmic-devel
Diff against target: 217 lines (+185/-0)
5 files modified
debian/changelog (+9/-0)
debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch (+49/-0)
debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch (+37/-0)
debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch (+87/-0)
debian/patches/series (+3/-0)
Reviewer Review Type Date Requested Status
Andreas Hasenack Approve
Canonical Server Pending
git-ubuntu developers Pending
Review via email: mp+360686@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Experimental PPA at https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3561/+packages

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

PPA verified with the testcase.
Bileto tests run on ticket: https://bileto.ubuntu.com/#/ticket/3561

This is open for review now ...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Discussions on the autookgtests in the Bionic MP at https://code.launchpad.net/~paelzer/ubuntu/+source/psmisc/+git/psmisc/+merge/360687

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

range-diff is nice for SRUs :)

git range-diff pkg/ubuntu/bionic-devel..lp1806060-fix-killall-option-parsing-bionic pkg/ubuntu/cosmic-devel..lp1806060-fix-killall-option-parsing-cosmic
1: f4614b0 = 1: ee79ceb d/patches/lp1806060-*: fix killall option parsing (LP: #1806060)
2: fbe4683 < -: ------- changelog: fix killall option parsing (LP: #1806060)
-: ------- > 2: 0028bb2 changelog: fix killall option parsing (LP: #1806060)

:)

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Ok:

- compared cosmic fix with bionic fix, same
- ran the test (nice script), ok
- even though sometimes the test script doesn't find a process to kill, by then it parsed the command line already, and if an unknown option were to be used it would have shown the help output
- killall is definitely used by many scripts out there, let's keep an eye on possible regressions

+1

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Forgot to say, also verified that upstream didn't have more follow-up commits with One More Fix.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks for the re-check Andreas.
Pushed Tag and uploaded for SRU

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 20cf8fd..8f784c8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
1psmisc (23.1-1ubuntu1.1) cosmic; urgency=medium
2
3 * fix killall option parsing (LP: #1806060)
4 - d/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch
5 - d/patches/lp1806060-0002-killall-change-to-getopt_long.patch
6 - d/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch
7
8 -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Tue, 11 Dec 2018 16:46:04 +0100
9
1psmisc (23.1-1build1) cosmic; urgency=medium10psmisc (23.1-1build1) cosmic; urgency=medium
211
3 * No-change rebuild for ncurses soname changes.12 * No-change rebuild for ncurses soname changes.
diff --git a/debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch b/debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch
4new file mode 10064413new file mode 100644
index 0000000..6bb7b59
--- /dev/null
+++ b/debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch
@@ -0,0 +1,49 @@
1From cf80d701397d2ff705ab1c1250e3906f32034539 Mon Sep 17 00:00:00 2001
2From: Craig Small <csmall@enc.com.au>
3Date: Sun, 20 May 2018 20:42:12 +1000
4Subject: [PATCH] killall: Fix -INT option parsing
5
6Sometimes killall gets confused between a signal name and a set of
7options. This small patch should un-confuse it.
8
9Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
10Original-Author: Craig Small <csmall@enc.com.au>
11Origin: backport, https://gitlab.com/psmisc/psmisc/commit/cf80d701397d2ff705ab1c1250e3906f32034539
12Bug-Ubuntu: https://bugs.launchpad.net/bugs/1806060
13Last-Update: 2018-12-11
14
15---
16 ChangeLog | 1 +
17 src/killall.c | 7 ++++++-
18 2 files changed, 7 insertions(+), 1 deletion(-)
19
20--- a/src/killall.c
21+++ b/src/killall.c
22@@ -803,6 +803,7 @@ main (int argc, char **argv)
23 int sig_num;
24 int optc;
25 int myoptind;
26+ int skip_error=0;
27 struct passwd *pwent = NULL;
28 char yt[16];
29 char ot[16];
30@@ -912,6 +913,7 @@ main (int argc, char **argv)
31 ignore_case = 1;
32 } else {
33 sig_num = get_signal (argv[optind]+1, "killall");
34+ skip_error=1;
35 }
36 break;
37 case 'V':
38@@ -947,7 +949,10 @@ main (int argc, char **argv)
39 if (argv[optind-1][1] >= '0' && argv[optind-1][1] <= '9') {
40 sig_num = atoi(argv[optind-1]+1);
41 } else {
42- usage(NULL);
43+ if (skip_error)
44+ skip_error=0;
45+ else
46+ usage(NULL);
47 }
48 }
49 break;
diff --git a/debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch b/debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch
0new file mode 10064450new file mode 100644
index 0000000..6eb56c3
--- /dev/null
+++ b/debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch
@@ -0,0 +1,37 @@
1From b769da1626044f97a689c1665270595f36f878e7 Mon Sep 17 00:00:00 2001
2From: Craig Small <csmall@enc.com.au>
3Date: Sun, 20 May 2018 20:46:55 +1000
4Subject: [PATCH] killall: change to getopt_long
5
6with getopt_long_only -ve fails because its not sure what the -v
7is for --version or --verbose. Changing to the non-_only version means
8it doesn't try to expand the options to long options.
9
10References:
11 psmisc/psmisc#12
12
13Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
14Original-Author: Craig Small <csmall@enc.com.au>
15Origin: backport, https://gitlab.com/psmisc/psmisc/commit/b769da1626044f97a689c1665270595f36f878e7
16Bug-Ubuntu: https://bugs.launchpad.net/bugs/1806060
17Last-Update: 2018-12-11
18
19---
20 ChangeLog | 1 +
21 src/killall.c | 4 ++--
22 2 files changed, 3 insertions(+), 2 deletions(-)
23
24--- a/src/killall.c
25+++ b/src/killall.c
26@@ -856,9 +856,9 @@ main (int argc, char **argv)
27
28 opterr = 0;
29 #ifdef WITH_SELINUX
30- while ( (optc = getopt_long_only(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
31+ while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
32 #else
33- while ( (optc = getopt_long_only(argc,argv,"egy:o:ilqrs:u:vwVIn:",options,NULL)) != -1) {
34+ while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwVIn:",options,NULL)) != -1) {
35 #endif
36 switch (optc) {
37 case 'e':
diff --git a/debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch b/debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch
0new file mode 10064438new file mode 100644
index 0000000..026f280
--- /dev/null
+++ b/debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch
@@ -0,0 +1,87 @@
1From 258ee9166e585f87005d3a9686938a4fa26669f9 Mon Sep 17 00:00:00 2001
2From: Craig Small <csmall@enc.com.au>
3Date: Tue, 18 Sep 2018 21:17:00 +1000
4Subject: [PATCH] killall: Another go at option parsing
5
6This now seems to be working. There are some evil hacks, especially for
7the -ve option combination but it seems that we have a winner.
8
9Added a bunch more option parsing tests which picked up -ILL passed but
10-VTALRM did not. Not sure why, but length seems my guess
11
12References:
13 psmisc/psmisc#13
14 psmisc/psmisc#12
15
16Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
17Original-Author: Craig Small <csmall@enc.com.au>
18Origin: backport, https://gitlab.com/psmisc/psmisc/commit/258ee9166e585f87005d3a9686938a4fa26669f9
19Bug-Ubuntu: https://bugs.launchpad.net/bugs/1806060
20Last-Update: 2018-12-11
21
22---
23 ChangeLog | 2 +-
24 src/killall.c | 20 ++++++++++++++------
25 testsuite/killall.test/killall.exp | 13 +++++++++++++
26 3 files changed, 28 insertions(+), 7 deletions(-)
27
28--- a/src/killall.c
29+++ b/src/killall.c
30@@ -856,7 +856,7 @@ main (int argc, char **argv)
31
32 opterr = 0;
33 #ifdef WITH_SELINUX
34- while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
35+ while ( (optc = getopt_long_only(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
36 #else
37 while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwVIn:",options,NULL)) != -1) {
38 #endif
39@@ -913,7 +913,7 @@ main (int argc, char **argv)
40 ignore_case = 1;
41 } else {
42 sig_num = get_signal (argv[optind]+1, "killall");
43- skip_error=1;
44+ skip_error=optind;
45 }
46 break;
47 case 'V':
48@@ -921,8 +921,10 @@ main (int argc, char **argv)
49 if (strcmp(argv[optind-1],"-V") == 0 || strncmp(argv[optind-1],"--",2) == 0) {
50 print_version();
51 return 0;
52+ } else {
53+ sig_num = get_signal (argv[optind]+1, "killall");
54+ skip_error=optind;
55 }
56- sig_num = get_signal (argv[optind]+1, "killall");
57 break;
58 case 'n':
59 opt_ns_pid = atoi(optarg);
60@@ -940,6 +942,15 @@ main (int argc, char **argv)
61 break;
62 #endif /*WITH_SELINUX*/
63 case '?':
64+ if (skip_error == optind)
65+ break;
66+ /* Sigh, this is a hack because -ve could be -version or
67+ * -verbose */
68+ if (strncmp(argv[optind-1], "-ve", 3) == 0) {
69+ verbose=1;
70+ exact=1;
71+ break;
72+ }
73 /* Signal names are in uppercase, so check to see if the argv
74 * is upper case */
75 if (argv[optind-1][1] >= 'A' && argv[optind-1][1] <= 'Z') {
76@@ -949,10 +960,7 @@ main (int argc, char **argv)
77 if (argv[optind-1][1] >= '0' && argv[optind-1][1] <= '9') {
78 sig_num = atoi(argv[optind-1]+1);
79 } else {
80- if (skip_error)
81- skip_error=0;
82- else
83- usage(NULL);
84+ usage(NULL);
85 }
86 }
87 break;
diff --git a/debian/patches/series b/debian/patches/series
index 4bbbe65..ad87743 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,4 @@
1manpage-fixes1manpage-fixes
2lp1806060-0001-killall-Fix-INT-option-parsing.patch
3lp1806060-0002-killall-change-to-getopt_long.patch
4lp1806060-0003-killall-Another-go-at-option-parsing.patch

Subscribers

People subscribed via source and target branches