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 _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
