[LinuxBIOS] patch: extending LAR, and removing elf from linuxbios (it is not needed)

[Uwe Hermann]

On Wed, Aug 29, 2007 at 09:20:05AM -0700, ron minnich wrote:
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>===================================================================

What's up with that "============" line?


> --- lib/lar.c	(revision 480)
> +++ lib/lar.c	(working copy)
> @@ -31,6 +31,13 @@
>  #define ntohl(x) (x)
>  #endif
>  
> +int run_address(void *f)
> +{
> +	int (*v) (void);
> +	v = f;
> +	return v();
> +}

Add a doxygen comment please.


>  int find_file(struct mem_file *archive, char *filename, struct mem_file *result)

Same here.

filename can be 'const char *filename' I guess.


>  {
>  	char *walk, *fullname;
> @@ -40,31 +47,108 @@
>  	printk(BIOS_SPEW, "LAR: Start %p len 0x%x\n", archive->start,
>  	       archive->len);
>  
> +	if (archive->len < sizeof(struct lar_header))
> +		printk(BIOS_ERR, "Error: truncated archive (%d bytes); minimum possible size is %d bytes\n", 
> +			archive->len, sizeof(struct lar_header));
> +
> +	/* Getting this for loop right is harder than it looks. All quantities are 
> +	  * unsigned. The LAR stretches from (e.g.) 0xfff0000 for 0x100000 
> +	  * bytes, i.e. to address ZERO. 
> +	  * As a result, 'walk', can wrap to zero and keep going (this has been 
> +	  * seen in practice). Recall that these are unsigned; walk can 
> +	  * wrap to zero; so, question, when is walk less than any of these:
> +	  * archive->start
> +	  * Answer: once walk wraps to zero, it is < archive->start
> +	  * archive->start + archive->len
> +	  * archive->start + archive->len  - 1
> +	  * Answer: In the case that archive->start + archive->len == 0, ALWAYS!
> +	  * A lot of expressions have been tried and all have been wrong. 
> +	  * So what would work? Simple:
> +	  * test for walk < archive->start + archive->len - 1 to cover the case
> +	  *	that the archive does NOT occupy ALL of the top of memory and 
> +	  *	wrap to zero; 
> +	  * and test for walk >= archive->start, 
> +	  * to cover the case that you wrapped to zero. 
> +	  * Unsigned pointer arithmetic that wraps to zero can be messy.
> +	  */
>  	for (walk = archive->start;
> -	     (walk - 1) < (char *)(archive->start + archive->len - 1 ); walk += 16) {
> +	     (walk < (char *)(archive->start + archive->len - sizeof(struct lar_header))) && 
> +			(walk >= (char *)archive->start); walk += 16) {
>  		if (strcmp(walk, MAGIC) != 0)
>  			continue;
>  
>  		header = (struct lar_header *)walk;
>  		fullname = walk + sizeof(struct lar_header);
>  
> -		printk(BIOS_SPEW, "LAR: current filename is %s\n", fullname);
> +		printk(BIOS_SPEW, "LAR: search for %s\n", fullname);
>  		// FIXME: check checksum
>  
>  		if (strcmp(fullname, filename) == 0) {
> +			printk(BIOS_SPEW, "LAR: CHECK %s @ %p\n", fullname, header);
>  			result->start = walk + ntohl(header->offset);
>  			result->len = ntohl(header->len);
>  			result->reallen = ntohl(header->reallen);
>  			result->compression = ntohl(header->compression);
> +			result->entry = (void *)ntohl(header->entry);
> +			result->loadaddress = (void *)ntohl(header->loadaddress);

Just curious, is the cast to (void *) really needed?


> +			printk(BIOS_SPEW, 
> +			"start %p len %d reallen %d compression %x entry %p loadaddress %p\n", 
> +				result->start, result->len, result->reallen, 
> +				result->compression, result->entry, result->loadaddress);
>  			return 0;
>  		}
>  		// skip file
>  		walk += (ntohl(header->len) + ntohl(header->offset) -
>  			1) & 0xfffffff0;
>  	}
> +	printk(BIOS_SPEW, "NO FILE FOUND\n");
>  	return 1;
>  }
>  
> +
> +void *load_file(struct mem_file *archive, char *filename)

Add doxygen comment, please.


> +{
> +	int ret;
> +	struct mem_file result;
> +	void *where;
> +	void *entry;
> +
> +	ret = find_file(archive, filename, &result);
> +	if (ret) {
> +		printk(BIOS_INFO, "LAR: load_file: No such file '%s'\n",
> +		       filename);
> +		return (void *)-1;

Uh? What is '(void *)-1' supposed to be? Will that work? Is the cast needed?


> +/* FIXME -- most of copy_file should be replaced by load_file */
>  int copy_file(struct mem_file *archive, char *filename, void *where)
>  {
>  	int ret;
> @@ -85,7 +169,7 @@
>  	}
>  #ifdef CONFIG_COMPRESSION_LZMA
>  	/* lzma */
> -	unsigned long ulzma(unsigned char * src, unsigned char * dst);
> +	unsigned long ulzma(unsigned char *src, unsigned char *dst);

ACK.


> @@ -130,9 +215,11 @@
>  		}
>  		where = result.start;
>  	}
> -
> +	printk(BIOS_SPEW, "where is %p\n", where);
>  	v = where;
> -	return v();
> +	ret =  v();
             ^^
Two spaces where only one should be.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070829/f9565a5c/attachment.sig>