[coreboot] `cbfstool remove` leaves empty file behind

[Aaron Durbin]

On Fri, Aug 7, 2015 at 12:30 AM, Paul Menzel
<paulepanter at users.sourceforge.net> wrote:
> Dear coreboot folks,
>
>
> with a coreboot image built from commit 11f1d31d (buildgcc: Deal with
> gmp on 32bit Cygwin on 64bit host) [1] for the ASRock E350M1 removing
> `fallback/payload` leaves an empty file behind and trying to add a file
> with the same name again fails, as cbfstool says it’s already present.
>
>     $ build/cbfstool build/coreboot.rom print
>     coreboot.rom: 4096 kB, bootblocksize 1576, romsize 4194304, offset 0x0
>     alignment: 64 bytes, architecture: x86
>
>     Name                           Offset     Type         Size
>     cmos_layout.bin                0x0        cmos_layout  1424
>     pci1002,9802.rom.lzma          0x5c0      optionrom    33379
>     fallback/dsdt.aml              0x8880     raw          9744
>     fallback/romstage              0xaf00     stage        189696
>     fallback/ramstage              0x39440    stage        125106
>     fallback/payload               0x57d40    payload      37033
>     config                         0x60e40    raw          3725
>     revision                       0x61d00    raw          569
>     etc/show-boot-menu             0x61f80    raw          8
>     (empty)                        0x61fc0    null         3791256
>     $ build/cbfstool build/coreboot.rom remove -n fallback/payload
>     $ build/cbfstool build/coreboot.rom add-payload -n fallback/payload -f seabios.elf -c lzma
>     E: 'fallback/payload' already in ROM image.
>     $ build/cbfstool build/coreboot.rom print
>     coreboot.rom: 4096 kB, bootblocksize 1576, romsize 4194304, offset 0x0
>     alignment: 64 bytes, architecture: x86
>
>     Name                           Offset     Type         Size
>     cmos_layout.bin                0x0        cmos_layout  1424
>     pci1002,9802.rom.lzma          0x5c0      optionrom    33379
>     fallback/dsdt.aml              0x8880     raw          9744
>     fallback/romstage              0xaf00     stage        189696
>     fallback/ramstage              0x39440    stage        125106
>     fallback/payload               0x57d40    null         37033
>     config                         0x60e40    raw          3725
>     revision                       0x61d00    raw          569
>     etc/show-boot-menu             0x61f80    raw          8
>     (empty)                        0x61fc0    null         3791256
>

If you notice, the type field changed from payload to null.

$ echo a > a
$ echo b > b
$ ./util/cbfstool/cbfstool  test.rom create -m x86 -s 1024
Created CBFS (capacity = 920 bytes)
$ ./util/cbfstool/cbfstool test.rom add -f a -n a -t raw
$ ./util/cbfstool/cbfstool test.rom add -f b -n b -t raw
$ cp test.rom test.rom.original
$ ./util/cbfstool/cbfstool test.rom remove  -n
$ diff -up <(hexdump -C test.rom.original) <(hexdump -C test.rom)
--- /dev/fd/63  2015-08-07 08:43:42.118430961 -0500
+++ /dev/fd/62  2015-08-07 08:43:42.114430961 -0500
@@ -1,4 +1,4 @@
-00000000  4c 41 52 43 48 49 56 45  00 00 00 02 00 00 00 50  |LARCHIVE.......P|
+00000000  4c 41 52 43 48 49 56 45  00 00 00 02 ff ff ff ff  |LARCHIVE........|
 00000010  00 00 00 00 00 00 00 28  61 00 00 00 00 00 00 00  |.......(a.......|
 00000020  00 00 00 00 00 00 00 00  61 0a ff ff ff ff ff ff  |........a.......|
 00000030  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
$ ./util/cbfstool/cbfstool test.rom add -f c -n c -t raw

$ ./util/cbfstool/cbfstool test.rom print
test.rom: 1 kB, bootblocksize 0, romsize 1024, offset 0x0
alignment: 64 bytes, architecture: x86

Name                           Offset     Type         Size
c                              0x0        raw          2
b                              0x40       raw          2
(empty)                        0x80       null         792

So it's "deteled" as the type changed. But the name wasn't changed to
match the '(empty)' heuristic.

The fix below it will do as you want (though remove doesn't really
clear contents...).

$ ./util/cbfstool/cbfstool test.rom remove  -n c
$ ./util/cbfstool/cbfstool test.rom print
test.rom: 1 kB, bootblocksize 0, romsize 1024, offset 0x0
alignment: 64 bytes, architecture: x86

Name                           Offset     Type         Size
(empty)                        0x0        null         2
b                              0x40       raw          2
(empty)                        0x80       null         792


diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index e095187..0f2f810 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -860,6 +860,9 @@ int cbfs_merge_empty_entry(struct cbfs_image *image, struct
                // Ready to be recycled.
                type = CBFS_COMPONENT_NULL;
                entry->type = htonl(type);
+               // Place NUL byte as first byte of name to be viewed as "empty".
+               uint8_t *name = (void *)&entry[1];
+               *name = '\0';
        }
        if (type != CBFS_COMPONENT_NULL)
                return 0;


Hope that helps.

-Aaron

>
> Thanks,
>
> Paul
>
>
> [1] http://review.coreboot.org/11123
> --
> coreboot mailing list: coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot