[Toybox] [CLEANUP] sysctl

Rob Landley rob at landley.net
Wed May 14 09:43:49 PDT 2014


Notes about commit 1283: http://landley.net/hg/toybox/rev/1283

In sysctl.c, trim_spaces() is only called twice, and it's basically:
   while(isspace(*start)) start++;
   for (len = strlen(start); isspace(start[len])); start[--len] = 0);

That's small enough that it's probably best to just have it inline so
you can see what the code's doing. (The duplication isn't worth the
obscurity.)

Limiting length to toybuf not sufficient.

Errors ignored by -e don't set exitval.

The values we read from the kernel have a trailing newline. The sysctl
code would be simpler if we just trusted that newline to be there, but I
strip it off and put it back on again because I don't want to rely on
the kernel doing that.

$ sysctl net.ipv6/route
error: "net.ipv6/route" is an unknown key

usage messages: replace [OPTIONS] with actual options, spaces to tabs in
the option descriptions.

Make the #defines go away. Yes, /proc/sys is occurs twice and the length
of it is hardwired in once, but it's never going to _change_. Factoring
it out doesn't reduce the actual number of occurrences of it in the
code, it just hides them.

The read/write error messages can just be "key '%s"' because we append
the errno message (which indicates whether a read or write failed, or if
it just didn't exist). The main reason for keeping it a function is to
factor out the ENOENT && FLAG_e test.

If I tried a bit harder I might be able to collapse stuff together so
there was only one instance of split_key() and key_error() and such, and
they could be inlined. But that doesn't stand in the way of promoting
this. (Still trying to catch up on pending...)

At this point the cleanup pass is complete from the design level, and I
just need to do a lot more testing before promoting it. (Alas, hard to
do an automated test suite because it wants root to change anything. And
I'm not _entirely_ sure what keys are portably there on all systems that
might want to run said test suite. Need to squint at it a bit...)

Rob


More information about the Toybox mailing list