First, a brief glance at the results of this patch: $ ~/runit-1.3.1.new/src/sv status -prf -s -w /local/ccd/runit-test/* /local/ccd/runit-test/foo 13274 exit:0 exit:2 finish up /local/ccd/runit-test/bar - signal:15 none:0 down down $ ~/runit-1.3.1.new/src/runsvstat /local/ccd/runit-test/* /local/ccd/runit-test/bar: run (pid 16819) 6 seconds /local/ccd/runit-test/foo: run (pid 16825) 4 seconds, last finish exit status 2 $ ~/runit-1.3.1.new/src/sv status /local/ccd/runit-test/* /local/ccd/runit-test/bar: run (pid 16884) 3 seconds; log: run (pid 16796) 73 seconds /local/ccd/runit-test/foo: run (pid 16883) 3 seconds, last finish exit status 2 (note that elements of the output format have been unified between "sv status" and "runsvstat"; this is because they no longer have two different chunks of code to do the same thing). Now, the bad news: This is not a small patch. While acquainting myself with runit's internals, I found their coding style such as to substantially reduce maintainability, and consequently did some refactoring on my way. There's still much to be done -- a great many of the items touched on in the rant pasted below are still issues, or are issues which have been addressed only in small portions of the code -- and frankly, I can appreciate why there would be some legitimate hesitation to apply a patch of this size, even if it *is* agreed that the changes it makes (beyond that for which it was begun) are necessary and proper. That said, the patch is attached... and the rant is below. Please take it with a grain of salt -- while I *am* a bit disappointed in the quality of runit's code, it's still an excellent piece of software when one is using, as opposed to reading, it. Oh, and all that said -- this patch needs testing before it goes into production. I've given it some attention myself, and will be handing it over to my QA department tomorrow (though there's no knowing when they'll actually have time to review it), but in any event more testing and desk-checking is certainly a Good Thing. ----------------------------------- Things This Patch Does: - Implements the previously-discussed flags to "sv status", tacking 4 bytes on to the end of the status file and updating some code to actually spit out the relevant information where useful. - Includes additional headers (and changes some types) to clean up warnings - Moves much duplicated code into a new "svdir.c"; eliminates duplicate parsing code, status->string generation code; etc. [Not to say that duplicate parsing code is -completely- eliminated; just from the places I touched. Need to go back and clean up the rest of it at some point]. - Widely uses "struct svdir" for storing status information, such that subprograms don't (need to) go picking around in status array. - Lets programs register their name to be printed with warnings (such that WARNING, ERROR and friends don't need to be redefined for each program and library methods can print errors which contain the current program's name) Things This Patch Doesn't Do: - Update the documentation - (?) ----------------------------------- The Rant (or, a bunch of questions I came up with while writing this thing): Why does runit use its own env_get() method rather than getenv()? [This is just one example; there are *lots* of places where C library calls are eschewed in favor of locally-implemented bits]. Why independently reimplement code to parse a status file FIVE SEPARATE TIMES (once each in runsv.c, runsvstat.c, sv.c, svwaitdown.c and svwaitup.c) rather than simply write a single library method and use that? Why define the same error methods and macros at the top of each file (as with fail/failx/warn/warnx/etc)? Why do sv.c and svstat.c both independently include code to build a string describing a service's status? Why the huge number of distinct error handling functions and macros with large numbers of parameters instead of just one or two functions using varargs? Why the structures with members named "x", "y" and "z" with no documentation anywhere describing what they are or do? Why the makefile that never heard of automatic dependency resolution or implicit rules? Why the obscure string constants? The one-letter variable names with no comments describing them? If one expects to get 3rd-party involvement in an open source project, it helps to actually have code that's of sufficient quality as to be *fun to work on*. Oh, and finally: Mixing tabs and whitespace is *abominable*. All tabs is good (so folks can have the amount of whitespace they want just by setting their editor, without needing to change the files). All whitespace is acceptable. Mixing tabs and whitespace, on the other hand, make an utter and complete *mess*. Bad! Evil!