chrt: Fix confusing error messages when priority argument is required

While making the priority argument optional for non-prio policies, some
confusing error messages were accidentally exposed to users for cases
that do need a priority argument.

  $ chrt true		#1
  chrt: invalid priority argument: 'true'
  $ chrt 1		#2
  chrt: failed to set pid 0's policy: Operation not permitted
  $ sudo chrt 1		#3
  chrt: no command specified
  $ chrt --other 1	#4
  chrt: unsupported priority value for the policy: 1: see --max for valid range

The error message #1 is caused by mixing `have_prio' and `need_prio'
together. Therefore, it always tries to parse the first positional
argument as a priority when `--pid' is not given. #2 shows that
set_sched() is meaninglessly called even when too few arguments are
specified.

At first glance, the error message #3 seems to be correct, but it turns
out to be very wrong -- the only positional argument in this case must
be regarded as a command, and commit 223a502b0208 ("chrt: (man,usage)
mark the priority value as optional in the synopses") also clearly
stated the same in the help message and the manual. In other words, #4
should have tried to execute `1' from PATH.

Fix #1 by decoupling `need_prio' from the priority parsing routine.

Fix #2 by parsing the first argument as a priority only when it's not
the only argument.

Fix #3 and #4 by consuming optind immediately when parsing priority
argument, instead of postponing it with inconsistent conditions (I have
an intuition that the previous code path was vibe-coded...)

Now #1 returns

  chrt: policy SCHED_RR requires a priority argument

... #2, #3 return

  chrt: no command or priority specified

... and #4 returns

  chrt: failed to execute 1: No such file or directory

... which are more sensible and helpful.

This doesn't break existing usage patterns:

  $ chrt --other true
  $ chrt --other 0 true
  $ chrt --other 1 true
  chrt: unsupported priority value for the policy: 1: see --max for valid range
  $ chrt --other echo meow
  meow

Fixes: 4c425142844d ("chrt: Allow optional priority for non‑prio policies without --pid")
Signed-off-by: Rong Zhang <i@rong.moe>

diff --git a/schedutils/chrt.c b/schedutils/chrt.c
index ab6e53ac6732246977e91048311cc5cc5024cbf1..5ba9964c1dd6c4465c2715fc5984c4841c432da0 100644 (file)
--- a/schedutils/chrt.c
+++ b/schedutils/chrt.c
@@ -510,9 +510,6 @@ int main(int argc, char **argv)
 
        /* If option --pid was given, parse the very last argument as a PID. */
        if (ctl->pid == 0) {
-               if (need_prio && argc - optind < 2)
-                       errx(EXIT_FAILURE, _("policy %s requires a priority argument"),
-                                               get_policy_name(ctl->policy));
                errno = 0;
                /* strtopid_or_err() is not suitable here, as 0 can be passed. */
                ctl->pid = strtos32_or_err(argv[argc - 1], _("invalid PID argument"));
@@ -530,13 +527,17 @@ int main(int argc, char **argv)
        if (ctl->pid > -1 && ctl->verbose)
                show_sched_info(ctl);
 
-       bool have_prio = need_prio ||
-               (ctl->pid == -1 ? (optind < argc && isdigit_string(argv[optind])) : (argc - optind > 1));
-
-       if (have_prio) {
+       if (argc - optind > 1 && (ctl->pid > -1 || isdigit_string(argv[optind]))) {
                errno = 0;
                ctl->priority = strtos32_or_err(argv[optind], _("invalid priority argument"));
-       } else
+
+               optind++;
+       } else if (need_prio && ctl->pid == -1 && isdigit_string(argv[optind]))
+               errx(EXIT_FAILURE, _("no command or priority specified"));
+       else if (need_prio)
+               errx(EXIT_FAILURE, _("policy %s requires a priority argument"),
+                                       get_policy_name(ctl->policy));
+       else
                ctl->priority = 0;
 
        if (ctl->runtime && !supports_runtime_param(ctl->policy))
@@ -576,11 +577,6 @@ int main(int argc, char **argv)
        if (!ctl->pid) {
                argv += optind;
 
-               if (need_prio)
-                       argv++;
-               else if (argv[0] && isdigit_string(argv[0]))
-                       argv++;
-
                if (argv[0] && strcmp(argv[0], "--") == 0)
                        argv++;