Patchwork [RESEND] cbfstool "add-lzma" method

This is a resend of a patch I sent last year.  Current distros ship a
version of lzma that does not set the "size" property of the
compressed archive.  This makes files compressed via the system
version of lzma unusable for coreboot and seabios.  This patch adds a
helper to cbfstool so users can easily add compressed "raw" files.

-Kevin

--------------------------- patch -----------------------

Enhance cbfstool so that it can support "cbfstool ROM add-lzma FILE
NAME" calls.  This is useful for callers that wish to add an lzma
compressed raw file.

Right now, SeaBIOS supports lzma raw files for things like floppy
images.  Today, adding one of these files requires a two step process
- for example:

$ lzma -zc /path/to/myfloppy.img > myfloppy.img.lzma
$ cbfstool coreboot.rom add myfloppy.img.lzma floppyimg/MyFloppy.lzma raw

Unfortunately, various versions of "lzma" are quirky and the above can
be troublesome.  With this patch, a user need only execute:

$ cbfstool coreboot.rom add-lzma myfloppy.img floppyimg/MyFloppy.lzma

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
---
 util/cbfstool/cbfs.h     |    1 +
 util/cbfstool/cbfstool.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++
 util/cbfstool/common.c   |    1 +
 3 files changed, 73 insertions(+), 0 deletions(-)

Dear Kevin,


Am Sonntag, den 16.01.2011, 13:23 -0500 schrieb Kevin O'Connor:
> This is a resend of a patch I sent last year.  Current distros ship a
> version of lzma that does not set the "size" property of the
> compressed archive.  This makes files compressed via the system
> version of lzma unusable for coreboot and seabios.  This patch adds a
> helper to cbfstool so users can easily add compressed "raw" files.
> 
> -Kevin
> 
> --------------------------- patch -----------------------
> 
> Enhance cbfstool so that it can support "cbfstool ROM add-lzma FILE
> NAME" calls.  This is useful for callers that wish to add an lzma
> compressed raw file.
> 
> Right now, SeaBIOS supports lzma raw files for things like floppy
> images.  Today, adding one of these files requires a two step process
> - for example:
> 
> $ lzma -zc /path/to/myfloppy.img > myfloppy.img.lzma
> $ cbfstool coreboot.rom add myfloppy.img.lzma floppyimg/MyFloppy.lzma raw
> 
> Unfortunately, various versions of "lzma" are quirky and the above can
> be troublesome.

could you give a hint what versions of `lzma` are quirky. Is that an
upstream problem and is this problem reported upstream?

> With this patch, a user need only execute:

s/a user need only execute:/a user only needs to execute:/ (?)

> $ cbfstool coreboot.rom add-lzma myfloppy.img floppyimg/MyFloppy.lzma
> 
> Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
> ---
>  util/cbfstool/cbfs.h     |    1 +
>  util/cbfstool/cbfstool.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++
>  util/cbfstool/common.c   |    1 +
>  3 files changed, 73 insertions(+), 0 deletions(-)
> 
> diff --git a/util/cbfstool/cbfs.h b/util/cbfstool/cbfs.h
> index 6fb9edd..44c1fa0 100644
> --- a/util/cbfstool/cbfs.h
> +++ b/util/cbfstool/cbfs.h
> @@ -73,6 +73,7 @@ struct cbfs_payload {
>  #define CBFS_COMPONENT_OPTIONROM  0x30
>  #define CBFS_COMPONENT_BOOTSPLASH 0x40
>  #define CBFS_COMPONENT_RAW        0x50
> +#define CBFS_COMPONENT_LZMA_RAW   0x40000050
>  #define CBFS_COMPONENT_VSA        0x51
>  #define CBFS_COMPONENT_MBI        0x52
>  #define CBFS_COMPONENT_MICROCODE  0x53
> diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
> index 507edc2..d437994 100644
> --- a/util/cbfstool/cbfstool.c
> +++ b/util/cbfstool/cbfstool.c
> @@ -18,6 +18,7 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
>   */
>  
> +#include <stdlib.h>

I do not know the coding style. Are headers sorted alphabetically?

>  #include <stdio.h>
>  #include <string.h>
>  #include "common.h"
> @@ -27,6 +28,7 @@ typedef enum {
>  	CMD_ADD,
>  	CMD_ADD_PAYLOAD,
>  	CMD_ADD_STAGE,
> +	CMD_ADD_LZMA,
>  	CMD_CREATE,
>  	CMD_LOCATE,
>  	CMD_PRINT,
> @@ -187,6 +189,73 @@ static int cbfs_add_stage(int argc, char **argv)
>  	return 0;
>  }
>  
> +static int cbfs_add_lzma(int argc, char **argv)
> +{
> +	char *romname = argv[1];
> +	char *cmd = argv[2];
> +	void *rom = loadrom(romname);
> +
> +	if (rom == NULL) {
> +		printf("Could not load ROM image '%s'.\n", romname);
> +		return 1;
> +	}
> +
> +	if (argc < 5) {
> +		printf("not enough arguments to '%s'.\n", cmd);

Capital letter at the start and s/to/to run/ (?).

Sorry if this is incorrect. I am no native speaker.

> +		return 1;
> +	}
> +
> +	char *filename = argv[3];
> +	char *cbfsname = argv[4];
> +
> +	uint32_t filesize = 0;
> +	void *filedata = loadfile(filename, &filesize, 0, SEEK_SET);
> +	if (filedata == NULL) {
> +		printf("Could not load file '%s'.\n", filename);
> +		return 1;
> +	}
> +
> +	uint32_t base = 0;
> +	void *cbfsfile = NULL;
> +
> +	uint32_t type = CBFS_COMPONENT_LZMA_RAW;
> +	if (argc > 5) {
> +		if (intfiletype(argv[5]) != ((uint64_t) - 1))
> +			type = intfiletype(argv[5]);
> +		else
> +			type = strtoul(argv[5], NULL, 0);
> +	}
> +	if (argc > 6) {
> +		base = strtoul(argv[6], NULL, 0);
> +	}
> +	uint32_t compresssize = filesize;
> +	void *compressed_data;
> +	for (;;) {
> +		compressed_data = malloc(compresssize);
> +		if (! compressed_data) {
> +			printf("Could not allocate %d space\n", compresssize);
> +			return 1;
> +		}
> +		uint32_t actualsize = compresssize;
> +		extern void do_lzma_compress(char *in, int in_len,
> +					     char *out, int *out_len);
> +		do_lzma_compress(filedata, filesize, compressed_data,
> +				 &compresssize);
> +		if (compresssize <= actualsize)
> +			break;
> +		// Try again with required size.
> +		free(compressed_data);
> +	}
> +
> +	cbfsfile = create_cbfs_file(cbfsname, compressed_data,
> +				    &compresssize, type, &base);
> +	if (add_file_to_cbfs(cbfsfile, compresssize, base))
> +		return 1;
> +	if (writerom(romname, rom, romsize))
> +		return 1;
> +	return 0;
> +}
> +
>  static int cbfs_create(int argc, char **argv)
>  {
>  	char *romname = argv[1];
> @@ -269,6 +338,7 @@ struct command commands[] = {
>  	{CMD_ADD, "add", cbfs_add},
>  	{CMD_ADD_PAYLOAD, "add-payload", cbfs_add_payload},
>  	{CMD_ADD_STAGE, "add-stage", cbfs_add_stage},
> +	{CMD_ADD_LZMA, "add-lzma", cbfs_add_lzma},
>  	{CMD_CREATE, "create", cbfs_create},
>  	{CMD_LOCATE, "locate", cbfs_locate},
>  	{CMD_PRINT, "print", cbfs_print},
> @@ -286,6 +356,7 @@ void usage(void)
>  	     " add FILE NAME TYPE [base address]    Add a component\n"
>  	     " add-payload FILE NAME [COMP] [base]  Add a payload to the ROM\n"
>  	     " add-stage FILE NAME [COMP] [base]    Add a stage to the ROM\n"
> +	     " add-lzma FILE NAME [TYPE] [base]     Lzma compress and add\n"
>  	     " create SIZE BOOTBLOCK [ALIGN]        Create a ROM file\n"
>  	     " locate FILE NAME ALIGN               Find a place for a file of that size\n"
>  	     " print                                Show the contents of the ROM\n"
> diff --git a/util/cbfstool/common.c b/util/cbfstool/common.c
> index 0fb0200..788d843 100644
> --- a/util/cbfstool/common.c
> +++ b/util/cbfstool/common.c
> @@ -142,6 +142,7 @@ struct filetypes_t {
>  	{CBFS_COMPONENT_OPTIONROM, "optionrom"},
>  	{CBFS_COMPONENT_BOOTSPLASH, "bootsplash"},
>  	{CBFS_COMPONENT_RAW, "raw"},
> +	{CBFS_COMPONENT_LZMA_RAW, "lzma-raw"},
>  	{CBFS_COMPONENT_VSA, "vsa"},
>  	{CBFS_COMPONENT_MBI, "mbi"},
>  	{CBFS_COMPONENT_MICROCODE, "microcode"},

I have not had time to test, so only

Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>


Thanks,

Paul