* [PATCH 0/6] pull: renice changes
@ 2014-09-06 11:24 Sami Kerola
2014-09-06 11:24 ` [PATCH 1/6] renice: reorder functions to avoid need of function prototype Sami Kerola
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Hello,
The renice(1) code looked a bit dated, so I decided to change it a bit
which resulted few more changes than I thought. The --priority <arg>
change makes the command line interface a bit less awkward, and the
numeric uid argument parsing is a bug fix. Rest of the changes are clean
ups.
The following changes since commit 3c2e64b0ca73b90b80365842e95abb2858217e60:
lsblk: add notes about udev to the man page (2014-09-05 10:02:45 +0200)
are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git renice
for you to fetch changes up to 13f618783fb0b8e19d2edef31074454fc4ad5a68:
rename: add getpriority() message lookup table (2014-09-06 12:07:10 +0100)
----------------------------------------------------------------
Sami Kerola (6):
renice: reorder functions to avoid need of function prototype
rename: use usage and version print out macros
renice: disallow --priority <arg> without pid argument
renice: avoid having same lines of code twice
renice: fix numeric uid argument parsing
rename: add getpriority() message lookup table
sys-utils/renice.c | 107 +++++++++++++++++++++++++++--------------------------
1 file changed, 54 insertions(+), 53 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] renice: reorder functions to avoid need of function prototype
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
@ 2014-09-06 11:24 ` Sami Kerola
2014-09-06 11:24 ` [PATCH 2/6] rename: use usage and version print out macros Sami Kerola
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 65 +++++++++++++++++++++++++++---------------------------
1 file changed, 32 insertions(+), 33 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index c0378e1..5d643fb 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -48,8 +48,6 @@
#include "c.h"
#include "closestream.h"
-static int donice(int,int,int);
-
static void __attribute__((__noreturn__)) usage(FILE *out)
{
fputs(_("\nUsage:\n"), out);
@@ -72,6 +70,38 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
+static int
+donice(int which, int who, int prio) {
+ int oldprio, newprio;
+ const char *idtype = _("process ID");
+
+ if (which == PRIO_USER)
+ idtype = _("user ID");
+ else if (which == PRIO_PGRP)
+ idtype = _("process group ID");
+
+ errno = 0;
+ oldprio = getpriority(which, who);
+ if (oldprio == -1 && errno) {
+ warn(_("failed to get priority for %d (%s)"), who, idtype);
+ return 1;
+ }
+ if (setpriority(which, who, prio) < 0) {
+ warn(_("failed to set priority for %d (%s)"), who, idtype);
+ return 1;
+ }
+ errno = 0;
+ newprio = getpriority(which, who);
+ if (newprio == -1 && errno) {
+ warn(_("failed to get priority for %d (%s)"), who, idtype);
+ return 1;
+ }
+
+ printf(_("%d (%s) old priority %d, new priority %d\n"),
+ who, idtype, oldprio, newprio);
+ return 0;
+}
+
/*
* Change the priority (the nice value) of processes
* or groups of processes which are already running.
@@ -155,34 +185,3 @@ main(int argc, char **argv)
return errs != 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}
-static int
-donice(int which, int who, int prio) {
- int oldprio, newprio;
- const char *idtype = _("process ID");
-
- if (which == PRIO_USER)
- idtype = _("user ID");
- else if (which == PRIO_PGRP)
- idtype = _("process group ID");
-
- errno = 0;
- oldprio = getpriority(which, who);
- if (oldprio == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
- return 1;
- }
- if (setpriority(which, who, prio) < 0) {
- warn(_("failed to set priority for %d (%s)"), who, idtype);
- return 1;
- }
- errno = 0;
- newprio = getpriority(which, who);
- if (newprio == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
- return 1;
- }
-
- printf(_("%d (%s) old priority %d, new priority %d\n"),
- who, idtype, oldprio, newprio);
- return 0;
-}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/6] rename: use usage and version print out macros
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
2014-09-06 11:24 ` [PATCH 1/6] renice: reorder functions to avoid need of function prototype Sami Kerola
@ 2014-09-06 11:24 ` Sami Kerola
2014-09-06 11:24 ` [PATCH 3/6] renice: disallow --priority <arg> without pid argument Sami Kerola
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 5d643fb..0605680 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -50,23 +50,20 @@
static void __attribute__((__noreturn__)) usage(FILE *out)
{
- fputs(_("\nUsage:\n"), out);
+ fputs(USAGE_HEADER, out);
fprintf(out,
_(" %1$s [-n] <priority> [-p|--pid] <pid>...\n"
" %1$s [-n] <priority> -g|--pgrp <pgid>...\n"
" %1$s [-n] <priority> -u|--user <user>...\n"),
program_invocation_short_name);
-
- fputs(_("\nOptions:\n"), out);
+ fputs(USAGE_OPTIONS, out);
fputs(_(" -g, --pgrp <id> interpret argument as process group ID\n"
" -n, --priority <num> specify the nice increment value\n"
" -p, --pid <id> interpret argument as process ID (default)\n"
" -u, --user <name|id> interpret argument as username or user ID\n"
" -h, --help display help text and exit\n"
" -V, --version display version information and exit\n"), out);
-
- fputs(_("\nFor more information see renice(1).\n"), out);
-
+ fprintf(out, USAGE_MAN_TAIL("renice(1)"));
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
@@ -129,9 +126,8 @@ main(int argc, char **argv)
if (strcmp(*argv, "-v") == 0 ||
strcmp(*argv, "-V") == 0 ||
strcmp(*argv, "--version") == 0) {
- printf(_("%s from %s\n"),
- program_invocation_short_name, PACKAGE_STRING);
- exit(EXIT_SUCCESS);
+ printf(UTIL_LINUX_VERSION);
+ return EXIT_SUCCESS;
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/6] renice: disallow --priority <arg> without pid argument
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
2014-09-06 11:24 ` [PATCH 1/6] renice: reorder functions to avoid need of function prototype Sami Kerola
2014-09-06 11:24 ` [PATCH 2/6] rename: use usage and version print out macros Sami Kerola
@ 2014-09-06 11:24 ` Sami Kerola
2014-09-06 11:24 ` [PATCH 4/6] renice: avoid having same lines of code twice Sami Kerola
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Earlier a lonely priority with an argument but without pid resulted to no
action and success, when the invocation should have failed.
$ renice --priority 42 ; echo $?
0
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 0605680..2075d40 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -131,14 +131,14 @@ main(int argc, char **argv)
}
}
- if (argc < 2)
- usage(stderr);
-
- if (strcmp(*argv, "-n") == 0 || strcmp(*argv, "--priority") == 0) {
+ if (*argv && (strcmp(*argv, "-n") == 0 || strcmp(*argv, "--priority") == 0)) {
argc--;
argv++;
}
+ if (argc < 2)
+ usage(stderr);
+
prio = strtol(*argv, &endptr, 10);
if (*endptr)
usage(stderr);
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] renice: avoid having same lines of code twice
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
` (2 preceding siblings ...)
2014-09-06 11:24 ` [PATCH 3/6] renice: disallow --priority <arg> without pid argument Sami Kerola
@ 2014-09-06 11:24 ` Sami Kerola
2014-09-12 7:35 ` Karel Zak
2014-09-06 11:24 ` [PATCH 5/6] renice: fix numeric uid argument parsing Sami Kerola
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Add getprio() function to avoid duplication of a simple task.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 2075d40..1c4b840 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -67,8 +67,21 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
-static int
-donice(int which, int who, int prio) {
+static int getprio(const int which, const int who, const char *idtype)
+{
+ int ret;
+
+ errno = 0;
+ ret = getpriority(which, who);
+ if (ret == -1 && errno) {
+ warn(_("failed to get priority for %d (%s)"), who, idtype);
+ return PRIO_MAX + 1;
+ }
+ return ret;
+}
+
+static int donice(int which, int who, int prio)
+{
int oldprio, newprio;
const char *idtype = _("process ID");
@@ -76,24 +89,14 @@ donice(int which, int who, int prio) {
idtype = _("user ID");
else if (which == PRIO_PGRP)
idtype = _("process group ID");
-
- errno = 0;
- oldprio = getpriority(which, who);
- if (oldprio == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
+ if ((oldprio = getprio(which, who, idtype)) == PRIO_MAX + 1)
return 1;
- }
if (setpriority(which, who, prio) < 0) {
warn(_("failed to set priority for %d (%s)"), who, idtype);
return 1;
}
- errno = 0;
- newprio = getpriority(which, who);
- if (newprio == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
+ if ((newprio = getprio(which, who, idtype)) == PRIO_MAX + 1)
return 1;
- }
-
printf(_("%d (%s) old priority %d, new priority %d\n"),
who, idtype, oldprio, newprio);
return 0;
@@ -103,8 +106,7 @@ donice(int which, int who, int prio) {
* Change the priority (the nice value) of processes
* or groups of processes which are already running.
*/
-int
-main(int argc, char **argv)
+int main(int argc, char **argv)
{
int which = PRIO_PROCESS;
int who = 0, prio, errs = 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] renice: fix numeric uid argument parsing
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
` (3 preceding siblings ...)
2014-09-06 11:24 ` [PATCH 4/6] renice: avoid having same lines of code twice Sami Kerola
@ 2014-09-06 11:24 ` Sami Kerola
2014-09-12 8:03 ` Karel Zak
2014-09-06 11:24 ` [PATCH 6/6] rename: add getpriority() message lookup table Sami Kerola
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
The following was inconflict with what usage() tells are valid option
arguments.
$ renice 1 -u 1000
renice: unknown user 1000
$ id
uid=1000(kerolasa) ...
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 1c4b840..1fef526 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -111,6 +111,7 @@ int main(int argc, char **argv)
int which = PRIO_PROCESS;
int who = 0, prio, errs = 0;
char *endptr = NULL;
+ const char *error_msg;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
@@ -162,18 +163,20 @@ int main(int argc, char **argv)
continue;
}
if (which == PRIO_USER) {
- register struct passwd *pwd = getpwnam(*argv);
+ struct passwd *pwd = getpwnam(*argv);
- if (pwd == NULL) {
- warnx(_("unknown user %s"), *argv);
- errs = 1;
- continue;
+ if (pwd != NULL)
+ who = pwd->pw_uid;
+ else {
+ error_msg = _("unknown user %s");
+ goto numeric_pid;
}
- who = pwd->pw_uid;
} else {
+ error_msg = _("bad value %s");
+ numeric_pid:
who = strtol(*argv, &endptr, 10);
if (who < 0 || *endptr) {
- warnx(_("bad value %s"), *argv);
+ warnx(error_msg, *argv);
errs = 1;
continue;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] rename: add getpriority() message lookup table
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
` (4 preceding siblings ...)
2014-09-06 11:24 ` [PATCH 5/6] renice: fix numeric uid argument parsing Sami Kerola
@ 2014-09-06 11:24 ` Sami Kerola
2014-09-16 20:21 ` [PATCH 0/6] pull: renice changes Sami Kerola
2014-09-22 12:48 ` Karel Zak
7 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-06 11:24 UTC (permalink / raw)
To: util-linux; +Cc: kerolasa
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 1fef526..a5bf50b 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -48,6 +48,12 @@
#include "c.h"
#include "closestream.h"
+const char *idtype[] = {
+ [PRIO_PROCESS] = N_("process ID"),
+ [PRIO_PGRP] = N_("process group ID"),
+ [PRIO_USER] = N_("user ID"),
+};
+
static void __attribute__((__noreturn__)) usage(FILE *out)
{
fputs(USAGE_HEADER, out);
@@ -67,38 +73,33 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
-static int getprio(const int which, const int who, const char *idtype)
+static int getprio(const int which, const int who)
{
int ret;
errno = 0;
ret = getpriority(which, who);
if (ret == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
+ warn(_("failed to get priority for %d (%s)"), who, idtype[which]);
return PRIO_MAX + 1;
}
return ret;
}
-static int donice(int which, int who, int prio)
+static int donice(const int which, const int who, const int prio)
{
int oldprio, newprio;
- const char *idtype = _("process ID");
- if (which == PRIO_USER)
- idtype = _("user ID");
- else if (which == PRIO_PGRP)
- idtype = _("process group ID");
- if ((oldprio = getprio(which, who, idtype)) == PRIO_MAX + 1)
+ if ((oldprio = getprio(which, who)) == PRIO_MAX + 1)
return 1;
if (setpriority(which, who, prio) < 0) {
- warn(_("failed to set priority for %d (%s)"), who, idtype);
+ warn(_("failed to set priority for %d (%s)"), who, idtype[which]);
return 1;
}
- if ((newprio = getprio(which, who, idtype)) == PRIO_MAX + 1)
+ if ((newprio = getprio(which, who)) == PRIO_MAX + 1)
return 1;
printf(_("%d (%s) old priority %d, new priority %d\n"),
- who, idtype, oldprio, newprio);
+ who, idtype[which], oldprio, newprio);
return 0;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] renice: avoid having same lines of code twice
2014-09-06 11:24 ` [PATCH 4/6] renice: avoid having same lines of code twice Sami Kerola
@ 2014-09-12 7:35 ` Karel Zak
2014-09-16 20:13 ` Sami Kerola
0 siblings, 1 reply; 15+ messages in thread
From: Karel Zak @ 2014-09-12 7:35 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sat, Sep 06, 2014 at 12:24:51PM +0100, Sami Kerola wrote:
> +static int getprio(const int which, const int who, const char *idtype)
> +{
> + int ret;
> +
> + errno = 0;
> + ret = getpriority(which, who);
> + if (ret == -1 && errno) {
> + warn(_("failed to get priority for %d (%s)"), who, idtype);
> + return PRIO_MAX + 1;
> + }
> + return ret;
> +}
This is exactly the situation when you want to use "return" only for
function status rather than for any data.
static int getprio(const int which,
const int who,
const char *idtype,
int *prio) <--------!
{
errno = 0;
*prio = getpriority(which, who);
if (*prio == -1 && errno) {
warn(_("failed to get priority for %d (%s)"), who, idtype);
return -errno;
}
return 0;
}
> - errno = 0;
> - newprio = getpriority(which, who);
> - if (newprio == -1 && errno) {
> - warn(_("failed to get priority for %d (%s)"), who, idtype);
> + if ((newprio = getprio(which, who, idtype)) == PRIO_MAX + 1)
> return 1;
> - }
if (getprio(which, who, idtype, &newprio) != 0)
return 1;
.. so you don't need complicated things like PRIO_MAX + 1.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] renice: fix numeric uid argument parsing
2014-09-06 11:24 ` [PATCH 5/6] renice: fix numeric uid argument parsing Sami Kerola
@ 2014-09-12 8:03 ` Karel Zak
2014-09-16 20:17 ` Sami Kerola
0 siblings, 1 reply; 15+ messages in thread
From: Karel Zak @ 2014-09-12 8:03 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sat, Sep 06, 2014 at 12:24:52PM +0100, Sami Kerola wrote:
> if (which == PRIO_USER) {
> - register struct passwd *pwd = getpwnam(*argv);
> + struct passwd *pwd = getpwnam(*argv);
>
> - if (pwd == NULL) {
> - warnx(_("unknown user %s"), *argv);
> - errs = 1;
> - continue;
> + if (pwd != NULL)
> + who = pwd->pw_uid;
> + else {
> + error_msg = _("unknown user %s");
> + goto numeric_pid;
> }
> - who = pwd->pw_uid;
> } else {
> + error_msg = _("bad value %s");
> + numeric_pid:
> who = strtol(*argv, &endptr, 10);
> if (who < 0 || *endptr) {
> - warnx(_("bad value %s"), *argv);
> + warnx(error_msg, *argv);
> errs = 1;
> continue;
> }
The goto is unnecessary. Just set who = -1 by default.
who = -1;
if (which == PRIO_USER) {
...
who = pwd->pw_uid;
}
if (who < 0) {
who = strtol(*argv, &endptr, 10);
...
if (who < 0 || *endptr)
warnx(_("failed to parse %s argument"), idtype[which]);
}
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] renice: avoid having same lines of code twice
2014-09-12 7:35 ` Karel Zak
@ 2014-09-16 20:13 ` Sami Kerola
0 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-16 20:13 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Fri, 12 Sep 2014, Karel Zak wrote:
> On Sat, Sep 06, 2014 at 12:24:51PM +0100, Sami Kerola wrote:
>> +static int getprio(const int which, const int who, const char *idtype)
>> +{
>> + int ret;
>> +
>> + errno = 0;
>> + ret = getpriority(which, who);
>> + if (ret == -1 && errno) {
>> + warn(_("failed to get priority for %d (%s)"), who, idtype);
>> + return PRIO_MAX + 1;
>> + }
>> + return ret;
>> +}
>
> This is exactly the situation when you want to use "return" only for
> function status rather than for any data.
>
> static int getprio(const int which,
> const int who,
> const char *idtype,
> int *prio) <--------!
> {
> errno = 0;
> *prio = getpriority(which, who);
> if (*prio == -1 && errno) {
> warn(_("failed to get priority for %d (%s)"), who, idtype);
> return -errno;
> }
> return 0;
> }
>
>
>> - errno = 0;
>> - newprio = getpriority(which, who);
>> - if (newprio == -1 && errno) {
>> - warn(_("failed to get priority for %d (%s)"), who, idtype);
>> + if ((newprio = getprio(which, who, idtype)) == PRIO_MAX + 1)
>> return 1;
>> - }
>
> if (getprio(which, who, idtype, &newprio) != 0)
> return 1;
>
> .. so you don't need complicated things like PRIO_MAX + 1.
Hi Karel,
That is a great improvement. Here is updated version of the change.
-->8---
From: Sami Kerola <kerolasa@iki.fi>
Date: Fri, 5 Sep 2014 23:33:16 +0100
Subject: [PATCH 4/7] renice: avoid having same lines of code twice
Add getprio() function to avoid duplication of a simple task.
Reviewed-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 2075d40..d83fc4a 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -67,8 +67,19 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
-static int
-donice(int which, int who, int prio) {
+static int getprio(const int which, const int who, const char *idtype, int *prio)
+{
+ errno = 0;
+ *prio = getpriority(which, who);
+ if (*prio == -1 && errno) {
+ warn(_("failed to get priority for %d (%s)"), who, idtype);
+ return -errno;
+ }
+ return 0;
+}
+
+static int donice(int which, int who, int prio)
+{
int oldprio, newprio;
const char *idtype = _("process ID");
@@ -76,24 +87,14 @@ donice(int which, int who, int prio) {
idtype = _("user ID");
else if (which == PRIO_PGRP)
idtype = _("process group ID");
-
- errno = 0;
- oldprio = getpriority(which, who);
- if (oldprio == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
+ if (getprio(which, who, idtype, &oldprio) != 0)
return 1;
- }
if (setpriority(which, who, prio) < 0) {
warn(_("failed to set priority for %d (%s)"), who, idtype);
return 1;
}
- errno = 0;
- newprio = getpriority(which, who);
- if (newprio == -1 && errno) {
- warn(_("failed to get priority for %d (%s)"), who, idtype);
+ if (getprio(which, who, idtype, &newprio) != 0)
return 1;
- }
-
printf(_("%d (%s) old priority %d, new priority %d\n"),
who, idtype, oldprio, newprio);
return 0;
@@ -103,8 +104,7 @@ donice(int which, int who, int prio) {
* Change the priority (the nice value) of processes
* or groups of processes which are already running.
*/
-int
-main(int argc, char **argv)
+int main(int argc, char **argv)
{
int which = PRIO_PROCESS;
int who = 0, prio, errs = 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] renice: fix numeric uid argument parsing
2014-09-12 8:03 ` Karel Zak
@ 2014-09-16 20:17 ` Sami Kerola
0 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-16 20:17 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
On Fri, 12 Sep 2014, Karel Zak wrote:
> On Sat, Sep 06, 2014 at 12:24:52PM +0100, Sami Kerola wrote:
>> if (which == PRIO_USER) {
>> - register struct passwd *pwd = getpwnam(*argv);
>> + struct passwd *pwd = getpwnam(*argv);
>>
>> - if (pwd == NULL) {
>> - warnx(_("unknown user %s"), *argv);
>> - errs = 1;
>> - continue;
>> + if (pwd != NULL)
>> + who = pwd->pw_uid;
>> + else {
>> + error_msg = _("unknown user %s");
>> + goto numeric_pid;
>> }
>> - who = pwd->pw_uid;
>> } else {
>> + error_msg = _("bad value %s");
>> + numeric_pid:
>> who = strtol(*argv, &endptr, 10);
>> if (who < 0 || *endptr) {
>> - warnx(_("bad value %s"), *argv);
>> + warnx(error_msg, *argv);
>> errs = 1;
>> continue;
>> }
>
> The goto is unnecessary. Just set who = -1 by default.
>
> who = -1;
> if (which == PRIO_USER) {
> ...
> who = pwd->pw_uid;
> }
>
> if (who < 0) {
> who = strtol(*argv, &endptr, 10);
> ...
> if (who < 0 || *endptr)
> warnx(_("failed to parse %s argument"), idtype[which]);
> }
Hi Karel,
Oh yes, I clearly over complicated the previous version. This ought to be
a bit closer what one could consider acceptable.
--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Fri, 5 Sep 2014 23:54:17 +0100
Subject: [PATCH 5/7] renice: fix numeric uid argument parsing
The following was inconflict with what usage() tells are valid option
arguments.
$ renice 1 -u 1000
renice: unknown user 1000
$ id
uid=1000(kerolasa) ...
Reviewed-by: Karel Zak <kzak@redhat.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index d83fc4a..a123ed1 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -160,14 +160,17 @@ int main(int argc, char **argv)
continue;
}
if (which == PRIO_USER) {
- register struct passwd *pwd = getpwnam(*argv);
+ struct passwd *pwd = getpwnam(*argv);
- if (pwd == NULL) {
+ if (pwd != NULL)
+ who = pwd->pw_uid;
+ else
+ who = strtol(*argv, &endptr, 10);
+ if (who < 0 || *endptr) {
warnx(_("unknown user %s"), *argv);
errs = 1;
continue;
}
- who = pwd->pw_uid;
} else {
who = strtol(*argv, &endptr, 10);
if (who < 0 || *endptr) {
@@ -180,4 +183,3 @@ int main(int argc, char **argv)
}
return errs != 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}
-
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] pull: renice changes
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
` (5 preceding siblings ...)
2014-09-06 11:24 ` [PATCH 6/6] rename: add getpriority() message lookup table Sami Kerola
@ 2014-09-16 20:21 ` Sami Kerola
2014-09-17 7:57 ` Benno Schulenberg
2014-09-22 12:48 ` Karel Zak
7 siblings, 1 reply; 15+ messages in thread
From: Sami Kerola @ 2014-09-16 20:21 UTC (permalink / raw)
To: util-linux
On Sat, 6 Sep 2014, Sami Kerola wrote:
> The renice(1) code looked a bit dated, so I decided to change it a bit
> which resulted few more changes than I thought. The --priority <arg>
> change makes the command line interface a bit less awkward, and the
> numeric uid argument parsing is a bug fix. Rest of the changes are clean
> ups.
Pull request I sent earlier is no longer completely up to date. The
renice branch is rebased on top of upstream master, and there is one new
change 0007: reorder usage() option descriptions.
The following changes since commit a3b92242ad76a7468cf508e1d878d0815c7e031f:
libmount: hide details about failed search in fstab/mtab (2014-09-16 15:30:03 +0200)
are available in the git repository at:
git://github.com/kerolasa/lelux-utiliteetit.git renice
for you to fetch changes up to a32b3f68da162eadbb17ff65a585d0249805c32f:
renice: reorder usage() option descriptions (2014-09-16 21:09:11 +0100)
----------------------------------------------------------------
Sami Kerola (7):
renice: reorder functions to avoid need of function prototype
rename: use usage and version print out macros
renice: disallow --priority <arg> without pid argument
renice: avoid having same lines of code twice
renice: fix numeric uid argument parsing
rename: add getpriority() message lookup table
renice: reorder usage() option descriptions
sys-utils/renice.c | 111
++++++++++++++++++++++++++---------------------------
1 file changed, 55 insertions(+), 56 deletions(-)
-->8---
From: Sami Kerola <kerolasa@iki.fi>
Date: Sun, 14 Sep 2014 11:12:58 +0100
Subject: [PATCH 7/7] renice: reorder usage() option descriptions
Make the Usage: and Options: sections to be in same order, which I found
to be quicker to use than alphabetical order.
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
sys-utils/renice.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/sys-utils/renice.c b/sys-utils/renice.c
index 994c9de..100f6a5 100644
--- a/sys-utils/renice.c
+++ b/sys-utils/renice.c
@@ -63,12 +63,13 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
" %1$s [-n] <priority> -u|--user <user>...\n"),
program_invocation_short_name);
fputs(USAGE_OPTIONS, out);
- fputs(_(" -g, --pgrp <id> interpret argument as process group ID\n"
- " -n, --priority <num> specify the nice increment value\n"
- " -p, --pid <id> interpret argument as process ID (default)\n"
- " -u, --user <name|id> interpret argument as username or user ID\n"
- " -h, --help display help text and exit\n"
- " -V, --version display version information and exit\n"), out);
+ fputs(_(" -n, --priority <num> specify the nice increment value\n"), out);
+ fputs(_(" -p, --pid <id> interpret argument as process ID (default)\n"), out);
+ fputs(_(" -g, --pgrp <id> interpret argument as process group ID\n"), out);
+ fputs(_(" -u, --user <name|id> interpret argument as username or user ID\n"), out);
+ fputs(USAGE_SEPARATOR, out);
+ fputs(USAGE_HELP, out);
+ fputs(USAGE_VERSION, out);
fprintf(out, USAGE_MAN_TAIL("renice(1)"));
exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] pull: renice changes
2014-09-16 20:21 ` [PATCH 0/6] pull: renice changes Sami Kerola
@ 2014-09-17 7:57 ` Benno Schulenberg
2014-09-17 10:06 ` Sami Kerola
0 siblings, 1 reply; 15+ messages in thread
From: Benno Schulenberg @ 2014-09-17 7:57 UTC (permalink / raw)
To: Sami Kerola; +Cc: Util-Linux
On Tue, Sep 16, 2014, at 22:21, Sami Kerola wrote:
> + fputs(_(" -u, --user <name|id> interpret argument as username or user ID\n"), out);
The argument here should be <name>|<id>.
(Think of <> as the // markers in email.)
Benno
--
http://www.fastmail.fm - The way an email service should be
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] pull: renice changes
2014-09-17 7:57 ` Benno Schulenberg
@ 2014-09-17 10:06 ` Sami Kerola
0 siblings, 0 replies; 15+ messages in thread
From: Sami Kerola @ 2014-09-17 10:06 UTC (permalink / raw)
To: Benno Schulenberg; +Cc: Util-Linux
On 17 September 2014 08:57, Benno Schulenberg <bensberg@justemail.net> wrote:
> On Tue, Sep 16, 2014, at 22:21, Sami Kerola wrote:
>> + fputs(_(" -u, --user <name|id> interpret argument as username or user ID\n"), out);
>
> The argument here should be <name>|<id>.
> (Think of <> as the // markers in email.)
Thank you for review Benno,
Update is in my git repo.
https://github.com/kerolasa/lelux-utiliteetit/commit/94a6c9b3cde3741ec1f7fe10cc3cead1a42990ae
--
Sami Kerola
http://www.iki.fi/kerolasa/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] pull: renice changes
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
` (6 preceding siblings ...)
2014-09-16 20:21 ` [PATCH 0/6] pull: renice changes Sami Kerola
@ 2014-09-22 12:48 ` Karel Zak
7 siblings, 0 replies; 15+ messages in thread
From: Karel Zak @ 2014-09-22 12:48 UTC (permalink / raw)
To: Sami Kerola; +Cc: util-linux
On Sat, Sep 06, 2014 at 12:24:47PM +0100, Sami Kerola wrote:
> Sami Kerola (6):
> renice: reorder functions to avoid need of function prototype
> rename: use usage and version print out macros
> renice: disallow --priority <arg> without pid argument
> renice: avoid having same lines of code twice
> renice: fix numeric uid argument parsing
> rename: add getpriority() message lookup table
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-09-22 12:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-06 11:24 [PATCH 0/6] pull: renice changes Sami Kerola
2014-09-06 11:24 ` [PATCH 1/6] renice: reorder functions to avoid need of function prototype Sami Kerola
2014-09-06 11:24 ` [PATCH 2/6] rename: use usage and version print out macros Sami Kerola
2014-09-06 11:24 ` [PATCH 3/6] renice: disallow --priority <arg> without pid argument Sami Kerola
2014-09-06 11:24 ` [PATCH 4/6] renice: avoid having same lines of code twice Sami Kerola
2014-09-12 7:35 ` Karel Zak
2014-09-16 20:13 ` Sami Kerola
2014-09-06 11:24 ` [PATCH 5/6] renice: fix numeric uid argument parsing Sami Kerola
2014-09-12 8:03 ` Karel Zak
2014-09-16 20:17 ` Sami Kerola
2014-09-06 11:24 ` [PATCH 6/6] rename: add getpriority() message lookup table Sami Kerola
2014-09-16 20:21 ` [PATCH 0/6] pull: renice changes Sami Kerola
2014-09-17 7:57 ` Benno Schulenberg
2014-09-17 10:06 ` Sami Kerola
2014-09-22 12:48 ` Karel Zak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).