[Toybox] chrt.c cleanup

[Rob Landley]

On 05/09/2017 10:57 AM, enh wrote:
> On Mon, May 8, 2017 at 4:38 PM, Rob Landley <rob at landley.net> wrote:
>> On 05/08/2017 03:52 PM, enh wrote:
>     toys/pending/chrt.c \

Ooh, it's <100 lines! Grab!

Huh, Ubuntu 14.04's headers don't have SCHED_DEADLINE. I wonder why? Hmmm...

  http://retis.sssup.it/~jlelli/talks/rts-like14/SCHED_DEADLINE.pdf

To use it I need two new system calls (sched_setattr() and
sched_getattr()), so it's not just adding -d for type 6 is it? (I
remember why this is still in pending!)

Right, skip that for now.

Option string has to start with ^ to stop at the first nonoption
argument (otherwise if you go "chrt ls -l" it'll try to parse -l for chrt).

Do you care if somebody does "chrt -m ls -l"? Right now -m happens and
it ignores the rest of the command line.

It's checking !*(toys.optargs+1) before checking *toys.optargs, meaning
if they go "chrt -p 42" that could be a wild pointer.

I hate the output strings (not unixy, hard to parse via script!) but
that's what Ubuntu's doing so compatibility trumps sanity...

Don't test !TT.pid, "chrt -p 0" queries the current PID. While we're at
it, add bounds checking so you can't chrt -p -1.

You're not outputting |SCHED_RESET_ON_FORK when querying via -p, the
other one does that.

Hmmm, given that other, batch, and idle are 0/0 in -m does it _need_ the
0? Yes it does. Can I do "chrt -b0 ls -l"? Nope, has to be separate.

Except... ARGH! The man page says "-p [prio] pid". Really? REALLY? If I
supply -p, then prio becomes optional (without it the thing queries
instead of sets) but it's not "-p pid" doing normal #*%(&# option
parsing, it's got pid _after_ the optional priority. That's not how
option parsing works! (Can I do the sane thing instead of the compatible
thing here? Pretty please?)

And... how do you indicate that in usage: syntax? Maybe...

    usage: chrt [-Rmofrbi] {-p PID [PRIORITY] | [PRIORITY COMMAND...]}

Can I do curly brackets to indicate a non-optional grouping? (I think
I've seen that. there isn't a spec here, and I don't do multiple usage:
lines because the help parser wouldn't know what to do with that.)

If you can printf("%ld", pid_t) then I can unwrap this bit of stupid
from bits/sched.h:

  /* The official definition.  */
  struct sched_param
  {
    int __sched_priority;
  };

(P.S. That two space indent is out of glibc!)

Except... glibc does that, bionic does that, the old uClibc I still have
lying around does that, _klibc_ does that... but for some insane reason
musl's include/sched.h has:

struct sched_param {
        int sched_priority;
        int sched_ss_low_priority;
        struct timespec sched_ss_repl_period;
        struct timespec sched_ss_init_budget;
        int sched_ss_max_repl;
};

Where did they get THAT from? Let's see... the kernel's
include/uapi/linux/sched/types.h has the "one int" version everybody
else has (that's where the got it from, I expect) and git annotate says
it was last touched in commit b7b3c76a0a21 in april 2006. So it's been
like that unchanged for 11 years.

Meanwhile musl's sched_getparam() is a straight syscall wrapper, so it's
_only_ setting the first field of this struct, meaning this is a musl
bug and I can ignore it and just use "int" here.

Why the toys.stacktop = 0 here? That forces xexec() to exec() rather
than running the built-in toybox version, is there a reason to do that here?

The ubuntu version is accepting "chrt -rp 12345" and ignoring the -r,
which tells it to _set_ the policy. (Which would require priority.) I
think I prefer to catch that and error instead...

Hmmm, this failed:

  $ sudo ./chrt -r -p 24532 3
  $ ./chrt -p 24532
  pid 24532's current scheduling policy: SCHED_RR
  pid 24532's current scheduling priority: 3
  $ sudo ./chrt -rR -p 24532 3
  chrt: 3 > -1

The report is from:

  atolx_range(*toys.optargs, sched_get_priority_min(policy),
    sched_get_priority_max(policy));

And it's complaining that sched_get_priority_min(policy) is returning
-1. Because even with the sudo, once the policy is set to SCHED_RR it
can't query the min/max range anymore? Ah, no: the problem is that if
you or in SCHED_RESET_ON_FORK _before_ calling the get_priorities(),
they return invalid argument.  Need to move the |= afterwards, then it
works.

I switched from linux/sched.h to bits/sched.h which provides _almost_
all the macros, except SCHED_RESET_ON_FORK. Which isn't there because I
_categorically_ refuse  to #define GNU_GNU_ALL_HAIL_STALLMAN for things
the Linux kernel developers invented. (Dear glibc developers: up yours,
you are wrong.) So that's 1<<30 and a comment. (I could add it to
portability.h, but eh.)

Rob