On 4/16/21 3:48 PM, enh wrote: > On Fri, Apr 16, 2021 at 11:44 AM Yi-yo Chiang <[email protected] > <mailto:[email protected]>> wrote: > > I'm not sure what Elliot's goal is? I assume he's trying to extract a > concatenated ramdisk, and I still see a problem in the current solution. > > > oops, i was going to say "i added you to the bug", but it looks like i added > you > to the *other* cpio bug (which appears to be resolving itself as "user error, > we'll fix our script"). i've added you > to https://issuetracker.google.com/184732694 now, but non-googlers won't be > able > to follow that link.
Yup, it's a login screen. > all i want to do is get my rug back^W^W^W^Wkeep someone's existing workflow > working. > > The buffer-format > > (https://www.kernel.org/doc/Documentation/early-userspace/buffer-format.txt) > says: > > initramfs := ("\0" | cpio_archive | cpio_gzip_archive)* > > In other words, both `cat a.cpio b.cpio >merged.cpio` and `(cat a.cpio && > echo -n -e '\0\0\0' && cat b.cpio) >merged.cpio` are valid initramfs. > > weird. do we know _why_ that's supported? Because gnu/cpio is crapping trailing nul bytes at the end of archives it creates: $ echo toys.h | cpio -o | hd | tail -n 5 2 blocks 000002c0 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 |................| 000002d0 0b 00 00 00 00 00 54 52 41 49 4c 45 52 21 21 21 |......TRAILER!!!| 000002e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00000400 So if you concatenate them, you get runs of nul bytes. > btw gen_init_cpio.c also pads initramfs to 512-byte boundary > > (https://github.com/torvalds/linux/blob/6fbd6cf85a3be127454a1ad58525a3adcf8612ab/usr/gen_init_cpio.c#L97) > > If we're viewing buffer-format.txt as the "right" cpio spec, then I think > we > should implement this too. We should skip arbitrary extra NUL-bytes padded > between cpio file frames > > not that it really matters (because i think we're all agreed that "do what the > kernel does" makes more sense than "do what GNU/BSD think") but does GNU cpio > cope with that? (see "strace" below for why i wonder.) > > > On Fri, Apr 16, 2021 at 2:27 PM Rob Landley <[email protected] > <mailto:[email protected]>> wrote: > > On 4/16/21 12:18 AM, Rob Landley wrote: > > On 4/15/21 6:57 PM, Rob Landley wrote: > >> On 4/15/21 12:06 PM, enh wrote: > >>> do you already have the "do the right thing" patch ready, or > should > i send that > >>> today? > >> > >> I'm good, I'll try to get it in tonight. The hard part is the test > suite, not > >> the implementation. > > > > Correction: the hard part is I don't seem to have actually > implemented > hardlink > > support yet. It just records/extracts them as separate files. > > > > Right. I need to test this against the kernel initramfs plumbing, > in both > > directions... > > if (!strcmp("TRAILER!!!", name)) { > + readall(afd, toybuf, 348); > > Where did you get 348 from? (What is this change doing?) > > I did what seems like a minimal change to continue past TRAILER!!! but > error on > an empty archive, but I apparently don't understand what your change > did? (I'm > assuming it fixed the use case you were seeing...?) > > yeah, from strace i think GNU cpio avoids this misalignment by always reading > 512-byte blocks (rather than toybox's "let me just read the header and then > worry about anything else later"). Because you can't unget into a nonseekable filehandle. > all i was trying to do was "get back in sync after a TRAILER!!! entry", and > 348 > (by measurement) got me back to 512 after what toybox cpio had read, for the > examples i had. (i had expected "is this guaranteed to be right for all > possible > valid cpio files?" to be the discussion we'd be having rather than the > "shouldn't we just do what the kernel does?" :-) ) > > (if i haven't said so already, assume i know nothing about cpio and have never > knowingly used it myself.) > > interestingly, the current patch seems to break the AOSP build: For some reason the github build test failed, although it ran fine here? And I couldn't view the actual logs on my phone because Microsoft Github gratuitously wanted me to log in for no obvious reason. (I never give my phone unnecessary credentials.) > 2021-04-16 18:56:56 - common.py - WARNING : Unable to get boot image build > props: Failed to run command '['toybox', 'cpio', '-F', > '/mnt/disks/build-disk/src/googleplex-android/sc-dev/out/soong/.temp/boot_xwvxvq26.img/uncompressed_ramdisk', > '-i']' (exit code 1): > cpio: bad header Hmmm... the 4 trailing NUL bytes on the "trailer" entry _should_ just put it back into alignment (strings are 4 byte aligned)? The old 6-digit magic field puts the header 2 bytes short, and then the 10 byte "TRAILER!?!" entry WOULD line up on 4 bytes except it should have a NUL terminator which puts it one over, thus needing 3 bytes of padding, so 4 trailing NUL bytes? > i'll chase up the person using `toybox cpio` and move them over to the > hermetic > prebuilt build toybox later. for now i'll try to reproduce this locally and > see > what the trouble is... Is this a _new_ failure, or is this a failure of the concatenated files it didn't handle before? (I.E. did my patch just not fix your issue since I don't have access to that test case to confirm I'd fixed it, or is it now more broken than it was?) Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
