[Toybox] [CLEANUP] cpio notes.

Rob Landley rob at landley.net
Wed Mar 26 03:39:03 PDT 2014


Commit 1227 at http://landley.net/hg/toybox/rev/1227 did lots of stuff:

main():
  - Don't need else "must use -iot" if argument parsing | handled it.

Query: how to deal with files > 2gb?
  - Which is worse: dying noisily or silently truncating them?

accept kernel directory format
output kernel listing format

Test: archive file with permission 000

main:
  Make -o be 1 so (toys.optflags & FLAG_o) returns stdin or stdout.
    - comment that, it's subtle.
  compress long runs of zeroes to %040X and similar in a sprintf to toybuf

Inline loopfiles_stdin()
  - Maybe we'll break it out again later for tar, but I want a version that
    can parse the kernel's full type/mode/owner/permissions/filename syntax,
    and this one isn't it. So inline it for now.

Inline write_cpio_member()
  - Caller uses "st" but argument is "buf", so rename "buf" to "st" in the
    code, and while we're there remove unnecessary parentheses after
    typecasts in header sprintf argument block.

Inline read_cpio_archive()
  - the fd argument is TT.afd, the how argument is toys.optflags.
    no need to pass globals.

Simplify htou():
  We can just use sscanf(). I'd inline it, but it needs error checking.
  (Because what if the data isn't hex digits?)
  And while we're at it, the header is just a big string. We don't need
  a structure for this (and I don't want the headache worrying about
  padding for portability, which probably isn't a problem here but I
  don't want to _explain_ it), so just feed "toybuf+offset" to htou.
  And really, we only ever parse 8 digits. So hardwire the new function
  reading 8 digits and returning an unsigned integer.

read_cpio_member:
  The arguments are globals, and "how" is basically just FLAG_t or not.
  (We don't get called if neither FLAG_t or FLAG_o was set.)

  We don't need the struct, we can just read into toybuf, treat it as
  a giant string (the fields aren't null terminated, just fixed length),
  and take a subset of the string for each field of interest.
  
Um, where did -d come from? The gnu/dammit version's man page hasn't
got it. Ah, it's in SUSv2. Except it's the default behavior these days,
like -u. Ok, accept -d and -u but ignore them, and don't bother to list
them in the help text. That means the option string exclusions become
[!io][!ot]

Remove the test for FLAG_d, and move the mkpathat() call into
the if() statement (-t should _not_ create directories).

error_msg() automatically sets exitval to 1 if it was 0, so we don't need
to do that. But we can perror_msg() to append the errno string. You don't
need \n for an error_msg().

Debug debug debug... Ah, we need to let symlink and file read their data
even for test, or the alignment is off. So redo the if statements to allow
that, and while we're at it add a -v flag to be verbose without disabling
output.

And every string we read has potential padding after it (name, symlink,
file), so break the string reading out into a function that does the
padding for us. Except the name starts at a 2 byte offset (header is 110
bytes), so add a TT.pad field to the globals. No, add an extra argument
to the function (which is 0 for all calls but the first, bit silly but...).

Make sure to always nul terminate the string, even if the cpio we're reading
didn't supply a trailing nul. (Never trust the input, assume it's trolling.)
Similarly, if there's no "TRAILER!!!" entry, parsing will still stop.
(With an error, but it'll stop.) And while we're at it, create all files
with O_NOFOLLOW. (Malicious archive could create a symlink and then
extract a file over it, rather not allow that.)

Hmmm, should I filter out "/../" components in archive? I could filter
each name through xabspath() and prefix it against xabspath(".")... and
xabspath() is misnamed because it doesn't die if there was a problem
(outside xgetcwd() and xmalloc() which are sort of incidental). That
should probably be called something else...

Test it! not working. what's wrong? scanf() doesn't like leading zeroes,
so the new x8u() needs to handle that part manually. Other random debugging...

Ok, I got -t working. Inline the read-side archive function the same
way we did the write side one, and zap the header struct (replace it with
a comment at the top describing the layout in 3 words).

Try extracting an archive, and... Segfault! Why did it segfault? Because I
fed the archive into -o, not into -i (oops). But it shouldn't segfault when
I do that, what did I miss?

Ok actually testing -i now. Increment "test" when calling error_msg()
for failure to mkpath() or open() so we don't 

Hmm, is xwrite() correct? It'll abort if we can't write data to a file.
Yeah, I think so. We continue if we can't _create_ a file (output an error
message and eventually return an error code, but otherwise soldier on),
but creating a _corrupted_ file isn't something we just want to gloss over.
That shouldn't happen normally, and tends to mean things like "disk full"
or "nfs go bye-bye" we can't really recover from anyway. (I was using it
in the first place because it'll retry short writes, now stopping to evaluate
if it's doing the right thing in the failure case...)

(Yes, baby talk is the correct way to refer to NFS.)

 1395830343.0


More information about the Toybox mailing list