Patchwork [U-Boot] bugfix: image header pointer change.

login
register
mail settings
Submitter Baidu Boy
Date Nov. 26, 2010, 11:59 p.m.
Message ID <AANLkTinEELvpV4s-ybN=xw-Pm7KFfpPw0yM=Ff_1pG_q@mail.gmail.com>
Download mbox | patch
Permalink /patch/73250/
State Superseded
Headers show

Comments

Baidu Boy - Nov. 26, 2010, 11:59 p.m.
images.legacy_hdr_os should point to the new copied structure,
not the original structure which may be over written.

Signed-off-by: Baidu Boy <liucai.lfn@gmail.com>
---
 common/cmd_bootm.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

 	case IMAGE_FORMAT_FIT:
Mike Frysinger - Nov. 27, 2010, 12:47 a.m.
On Friday, November 26, 2010 18:59:42 Baidu Boy wrote:
> -		show_boot_progress (6);
> +		show_boot_progress (6);

this doesnt seem to actually be changing anything.  you should avoid unrelated 
syntax changes with functional changes.
-mike
Baidu Boy - Nov. 27, 2010, 1:08 a.m.
Hi,Mike:
the purpose of below code is to copy the image header to the static
variable images.legacy_hdr_os_copy, and then the images.legacy_hdr_os
points to the
image header in  the static variable images.
------
		memmove (&images->legacy_hdr_os_copy, hdr, sizeof(image_header_t));

		/* save pointer to image header */
		images->legacy_hdr_os = hdr;
------
but the original code still lets the mages->legacy_hdr_os to point to
the header of uImage which may be overwritten in the decompress
progress.
That is not what we want.

Thanks


2010/11/27 Mike Frysinger <vapier@gentoo.org>:
> On Friday, November 26, 2010 18:59:42 Baidu Boy wrote:
>> -             show_boot_progress (6);
>> +             show_boot_progress (6);
>
> this doesnt seem to actually be changing anything.  you should avoid unrelated
> syntax changes with functional changes.
> -mike
>
Mike Frysinger - Nov. 27, 2010, 2:33 a.m.
On Friday, November 26, 2010 20:08:07 Baidu Boy wrote:
> Hi,Mike:

please do not top post

> the purpose of below code is to copy the image header to the static

please read the exact text i quoted
-mike
Baidu Boy - Nov. 27, 2010, 3:03 a.m.
I get your point. So I need to re-send a patch?

2010/11/27 Mike Frysinger <vapier@gentoo.org>:
> On Friday, November 26, 2010 20:08:07 Baidu Boy wrote:
>> Hi,Mike:
>
> please do not top post
>
>> the purpose of below code is to copy the image header to the static
>
> please read the exact text i quoted
> -mike
>
Mike Frysinger - Nov. 27, 2010, 3:39 a.m.
On Friday, November 26, 2010 22:03:26 Baidu Boy wrote:
> I get your point. So I need to re-send a patch?

again, please stop top posting.  it'd probably be good to send a new patch 
that didnt touch the boot progress line.
-mike

Patch

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 1a024f1..9ad5b19 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -896,10 +896,9 @@  static void *boot_get_kernel (cmd_tbl_t *cmdtp,
int flag, int argc, char * const
 		memmove (&images->legacy_hdr_os_copy, hdr, sizeof(image_header_t));

 		/* save pointer to image header */
-		images->legacy_hdr_os = hdr;
-
+		images->legacy_hdr_os = &images->legacy_hdr_os_copy;
 		images->legacy_hdr_valid = 1;
-		show_boot_progress (6);
+		show_boot_progress (6);
 		break;
 #if defined(CONFIG_FIT)