[Toybox] [PATCH] Add the gzip/gunzip/zcat I wrote for toolbox.
enh
enh at google.com
Mon May 8 18:27:59 PDT 2017
On Mon, May 8, 2017 at 4:37 PM, Rob Landley <rob at landley.net> wrote:
> On 04/26/2017 05:03 PM, enh wrote:
>> if you're actually going to start to look, i'll attach my port of the
>> current toolbox implementation.
>
> Ok, carving out a half hour to look at this... optargs should really
> have [-123456789] at the end so "gzip -3 -9" is -9 but "gzip -9 -3" is
> -3. While we're at it, OLDTOY() was there so these things could share
> option strings, except why does zcat have -cd?
i just copied what the "real thing" does --- all three commands accept
the same set of options, and just ignore the ones they can't use. i
assumed _someone_ is relying on that. (i can't remember if i actually
hit such an instance --- i don't remember how it was that i realized
that they all accept all the options, but i do remember in the
beginning i had a different getopt string for each one, but changed
that even in the toolbox version.)
> (And the three main
> functions... if oldtoy isn't handling this right it needs fixing. Do you
> care about the standalone builds, by the way? that was always the
> complicating part, chording together shared infrastructure...)
i don't personally understand why someone would want one of these but
not all three, no.
> I need to spend a 3 day weekend fixing the help infrastructure so it can
> do reasonable includes, the duplication of -c and -f help text here
> pains me.
the fact that the ps/top/pgrep help is just wrong pains me more :-)
note that -f isn't the same between all three, and the -c is slightly
different for all three. (and they'd differ more if we behaved more
like the "real thing" and documented accurately.)
> Ok, jumping down to main()... it seems like there should be a
> first_bit() function in lib/lib.c. Throw it in locally and move it when
> I find a second user. (Leaving it _not_ static to remind me about the
> library collation next time I hunt around for non-static functions.)
>
> Setting FLAG_c for no arguments can't be right, assuming gzip is doing
> what I think it is...
>
> gunzip one.gz - three.gz < two.gz
>
> And yes two gets output to stdout. So gunzip - writes "back" to stdout
> automatically, so the loopfiles() behavior of "no arguments means one
> argument equivalent to -" is fine and the callback has to handle that.
>
> loopfiles() second argument is a function pointer, you don't need a
> do_gz() wrapper you can just have the test in loopfiles to pass in the
> right argument. (It's not changing partway through the loop...)
>
> Hmmm, there's plumbing in things like patch and sed -i using
> copy_tempfile() to create a temporary file we write to (and delete if
> we're interrupted, via an atexit() variant). You're leaving the
> half-finished output file if we get interrupted, is that what we should do?
>
> Also, some commands go to a bit of trouble _not_ to exit immediately if
> they hit an error in one of the arguments, but instead produce an error
> message and continue on to the next argument. (This is why error_msg()
> sets toys.exitval to 1 if it's not already set, so that when we _do_
> eventually exit it records the error.)
>
> Should gzip do that? You have it exiting immediately... Huh, it looks
> like the gnu/gnu/gnu/dammit version _does_ exit at the first error,
> which seems awkward and wrong. So this matches the existing version, the
> question remains what the behavior _should_ be.
in the absence of a strong reason to do anything different, i just did
what i observed the "real thing" doing.
afaik the only differences are omissions.
> And I'm over my half hour with half the file left to go. And I haven't
> even spliced in the deflate plumbing from compress yet. :)
>
> Rob
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
More information about the Toybox
mailing list