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

[Rob Landley]

Checked in the rest of the cpio cleanup yesterday, now I need to make
sure I've addressed the issues from Isaac's two patches before promoting it.

(Ideally I'd like a proper scripts/test/cpio.test, but I'm not sure what
it should look like. The easy way to test cpio is against other cpio
implementations, and the test suite is designed to be self-contained.
Maybe I should include some small canned archives? But creating device
nodes requires root access...)

Longer-term I still want to teach this to handle the kernel's cpio
creation syntax, ala:

  dir /dev 755 0 0
  nod /dev/console 644 0 0 c 5 1
  nod /dev/loop0 644 0 0 b 7 0
  dir /bin 755 1000 1000
  slink /bin/sh busybox 777 0 0
  file /bin/busybox initramfs/busybox 755 0 0
  dir /proc 755 0 0
  dir /sys 755 0 0
  dir /mnt 755 0 0
  file /init initramfs/init.sh 755 0 0

It should be able to both consume that (ala gen_init_cpio) and produce
it (ala gen_initramfs_list.sh). But that's not blocking promotion.

Let's see:

On 03/16/14 22:09, ibid.ag at gmail.com wrote:
> No infinite loop on EOF; archive devices; allow -it

Sorry i didn't get to this patch promptly but I was still cleaning cpio
up, and by the time I got the email the code I was working on didn't
look much like what was in source control. :)

> 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.

The current stuff is checking getline's return code and <1 is considered
EOF. So 0 length line (not even a newline) or error (which wouldn't be
reported but it's the file list so I'm not sure what we'd say). So I
think that one's fixed.

> Non-regular files are archived as size 0; whether we can open them
> is irrelevant.
> The test most exactly expressing the logic would be
> if (lstat(...) || ((S_ISREG(...)) && (open(...)<0))
>     //skip this file verbosely

Hmmm, good point. Adjusted.

> 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.

It's all inline now, but gcc still complains that close() is called witn
an uninitialized fd. Which is true. (The awkwardness is I'm trying to
get two cases to share the same error output, without just sticking a
goto in there.)

What I can do is initialize fd to -1, then passing that to close should
be a NOP. (I mean, it's an error and sets errno, but we don't care. It's
cheap and should never do anything _bad_.)

> 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 
> 
> which should output: 
> /dev/null
> /dev/console
> 
> (the cpio you're testing must be in $PATH)

Yeah, I should do a test suite. Lemme get to your second patch first. :)

> HTH,
> Isaac Dunham
> 
> 

(Life would be _so_ much easier if gmail didn't "help out" by deleting
the second copy of list emails I get cc'd on, so messages I'm not cc'd
on wind up in the "toybox" folder and messages I am cc'd on wind up in
my inbox but _not_ the toybox folder. Google has a very strong
hallucination about what reality isn't like, and will impose it with hot
irons if necessary.)

Rob

 1395749849.0