Patchwork CBFS decompress error

login
register
about
Submitter Myles Watson
Date 2009-10-08 20:50:08
Message ID <2831fecf0910081350m78869100w106a3404e35b3326@mail.gmail.com>
Download mbox | patch
Permalink /patch/372/
State Accepted
Headers show

Comments

Myles Watson - 2009-10-08 20:50:08
On Thu, Oct 8, 2009 at 2:49 PM, Myles Watson <mylesgw@gmail.com> wrote:
> Move the ulzma prototype into cbfs.h
> Check the return value when decompressing.

With the patch.
Signed-off-by: Myles Watson <mylesgw@gmail.com>

> It's not 100% fixed, since the caller (cbfs_load_stage)
>
>        if (cbfs_decompress(stage->compression,
>                             ((unsigned char *) stage) +
>                             sizeof(struct cbfs_stage),
>                             (void *) (u32) stage->load,
>                             stage->len))
>                return (void *) -1;
>
> returns -1 and then cbfs_and_run_core jumps to it.
>
> print_debug("Jumping to image.\r\n");
> dst = cbfs_load_stage(filename);
> print_debug("Jumping to image.\r\n");
>
> Why do we jump to a -1 instead of hanging?
>
> Thanks,
> Myles
>
Peter Stuge - 2009-10-08 20:57:17
Myles Watson wrote:
> +	case CBFS_COMPRESS_LZMA:
> +		if (!ulzma(src, dst)) {
> +			printk_err("CBFS: ulzma returned 0!\n");
> +			return -1;
> +		}

Please say "ulzma() failed!" or maybe "LZMA decompression failed!" ?

Other than that,

Acked-by: Peter Stuge <peter@stuge.se>
Myles Watson - 2009-10-08 21:07:09
On Thu, Oct 8, 2009 at 2:57 PM, Peter Stuge <peter@stuge.se> wrote:
> Myles Watson wrote:
>> +     case CBFS_COMPRESS_LZMA:
>> +             if (!ulzma(src, dst)) {
>> +                     printk_err("CBFS: ulzma returned 0!\n");
>> +                     return -1;
>> +             }
>
> Please say "ulzma() failed!" or maybe "LZMA decompression failed!" ?
Good point.  Fixed.

Thanks,
Myles
Myles Watson - 2009-10-09 15:23:38
> Please say "ulzma() failed!" or maybe "LZMA decompression failed!" ?
>
> Other than that,
>
> Acked-by: Peter Stuge <peter@stuge.se>

With your suggestion and Patrick's taken into account.

Rev 4752.

Thanks,
Myles

Patch

Index: svn/src/lib/cbfs.c
===================================================================
--- svn.orig/src/lib/cbfs.c
+++ svn/src/lib/cbfs.c
@@ -36,15 +36,15 @@  int cbfs_decompress(int algo, void *src,
 		memcpy(dst, src, len);
 		return 0;
 
-	case CBFS_COMPRESS_LZMA: {
-		unsigned long ulzma(unsigned char *src, unsigned char *dst);
-		ulzma(src, dst);
-	}
+	case CBFS_COMPRESS_LZMA:
+		if (!ulzma(src, dst)) {
+			printk_err("CBFS: ulzma returned 0!\n");
+			return -1;
+		}
 		return 0;
 
 	default:
-		printk_info( "CBFS:  Unknown compression type %d\n",
-		       algo);
+		printk_info( "CBFS:  Unknown compression type %d\n", algo);
 		return -1;
 	}
 }
Index: svn/src/include/cbfs.h
===================================================================
--- svn.orig/src/include/cbfs.h
+++ svn/src/include/cbfs.h
@@ -50,14 +50,17 @@ 
 #define _CBFS_H_
 
 #include <boot/coreboot_tables.h>
+
 /** These are standard values for the known compression
     alogrithms that coreboot knows about for stages and
-    payloads.  Of course, other LAR users can use whatever
+    payloads.  Of course, other CBFS users can use whatever
     values they want, as long as they understand them. */
 
 #define CBFS_COMPRESS_NONE  0
 #define CBFS_COMPRESS_LZMA  1
 
+unsigned long ulzma(unsigned char *src, unsigned char *dst);
+
 /** These are standard component types for well known
     components (i.e - those that coreboot needs to consume.
     Users are welcome to use any other value for their