[Toybox] cpio: allow -it and fix two bugs in -o

[Rob Landley]

I note I'm in the middle of cpio cleanup. :)

On 03/16/14 19:35, ibid.ag at gmail.com wrote:
> No infinite loop on EOF; archive devices; allow -it
>
> getline(&name, &size, fd) will return -1 at EOF without setting
> name to 0. Thus, the old check resulted in an infinite loop 
> re-archiving the file.

Blah. I changed it from reallocating the buffer each time through to
reusing the existing buffer, but yeah, that screws up the error
reporting. I'll fix it, thanks.

The real problem is I don't ahve a test setup for this yet. I plan to
add support for the initramfs generation syntax so I can specify dev
nodes and such as a normal user, and that way I can test the full range
of cpio archive creation without root access. (Right now I can't put it
in scripts/test because I can't mknod as a normal user.)

> Non-regular files are archived as size 0; whether we can open them
> is irrelevant.

Except that the code I currently have checked in considers the inability
to open them an error. (I have a todo item to fix that.)

> The test most exactly expressing the logic would be
> if (lstat(...) || ((S_ISREG(...)) && (open(...)<0))
>     //skip this file verbosely

Indeed.

> But GCC thinks fd may be used uninitialized here, which is
> technically half-correct; in the case of a non-regular file, 
> fd would be passed to write_cpio_member uninitialized, and then ignored.
> So instead, we ignore a failed open() on non-regular files.
> 
> SUSv2 says to use -it not -t; don't prevent it.

I fixed that locally already, dunno if I checked it in yet...

> --
> I discovered that there were a few bugs in the new archive creation code.

Not at all surprised yet, I'm not done and haven't tested it much yet.

> Besides the two this fixes, the padding is very much wrong.
> I have not (yet) fixed that.
> 
> The comment got stuck in there because the check has gotten deleted
> once, so it seems to not be obvious what it's doing.
> 
> I've started a test, but...it needs some more work.
> A quick check is:
> echo -e "/dev/null\n/dev/console"| cpio -o | cpio -it 

I can use existing standard /dev nodes in the test suite, but I can't
_extract_ the result as a normal user. (I can at least list it, so
that's somthing.) And the problem with testing an archvier is creating
and extracting with the same tool doesn't prove I did it right. (I can
check against an sha1sum of a known archive, but that's brittle;
changing timestamps break it. I suspect I have to break down and do a
test-as-root to get reasonable coverage. I guess I can stick those at
the end...)

The real tests are against other implementations, both the gnu/dammit
one (it creates archives we extract, it extracts the archives we
create), and the kernel's initramfs stuff. (We can extract the stuff
they generate, initramfs can boot from archives we create.)

I didn't quite get to a good stopping point on today's cleanup (weekends
aren't dedicated programming time, they're just when I can carve some
out), but if I get up when my 5 am alarm goes off I'll try to get to the
next stopping point on cpio and check something in.

(Slightly longer-term, like to change my aboriginal initramfs generation
code to use toybox's cpio instead of the kernel's built-in stuff. But
probably not tomorrow morning. :)

Thanks,

Rob

 1395031695.0