On Wed, Aug 29, 2007 at 09:20:05AM -0700, ron minnich wrote: > Signed-off-by: Ronald G. Minnich <[EMAIL > PROTECTED]>===================================================================
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
signature.asc
Description: Digital signature
-- linuxbios mailing list [email protected] http://www.linuxbios.org/mailman/listinfo/linuxbios
