[U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu

[Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com>]

Stefan Roese wrote:
> Hi Timur,
> 
> On Friday 04 May 2007 15:25, Timur Tabi wrote:
>> I'm glad you fixed the bug, so I'll just add a few comments:
>>> Your comment is actually wrong. The use of "len" is not limited to
>>> that purpose.
>> If you apply my patch, then the comment becomes correct.  My goal was to
>> lock the variables 'len' and 'data' into one purpose.  The reason the
>> bug existed is because the other developer didn't realize this.  He used
>> 'len' thinking it was available.  In a sense, I was trying to implement
>> some "defensive programming", so that the next time someone hacks up
>> do_bootm_linux(), he won't re-introduce the bug.
>>
>> Now, you might say that that won't happen again, but I disagree.  I
>> think it can, for two reasons:
>>
>> 1) It happened once already, last year.  You approved and applied a
>> patch which does overwrite the variable.
>>
>> 2) The libfdt code introduced a number of other bugs relating to dtu
>> usage, which have not yet been fixed.
>>
>> So I believe there is a real possibility that another patch could
>> re-introduce this bug.  If you had applied my patch as-is, that
>> possibility would have been eliminated.  This is why I think my patch is
>> better than yours.
>>
>> But I guess only time will tell who's right. :-)
> 
> You have a point with your variable usage "restriction", and Wolfgang has a 
> point with the "readability" of your patch as it doesn't really tell what is 
> fixed without read the current source. Could be that your implementation is 
> the "better" one, but the patch Wolfgang applied was just less intrusive. 
> 
>>> And please accept my apologies thatt his was so complicated and  took
>>> so  long.  [Nevertheless you still might want to try to find a way to
>>> access the repository I  created  for  you  in  case  you  have  more
>>> patches.]
>> Stefan said he had a testing repo of some kind.  How about we just use
>> that?  If Stefan is willing, he can apply my emailed patches to his repo.
> 
> No, we shouldn't "fork" the code here. Let's focus on the other open issues. 
> You mention other bugs introduced by the libfdt code above. ;-)
> 
> Best regards,
> Stefan

Timur has sent me two libfdt/dtu-related fixes
* fdt_copy_into() had the source and destination addresses reversed
* fdt_check_header() had the wrong sense on ==/!= 0
which I applied to my local repo last night and will push back to 
u-boot-fdt soon (I still have not been successful in figuring out how to 
make a multi-image to test the changes grrrr).  The two bugs I 
introduced in the conversion to libfdt were unrelated to the "len" 
variable, but I probably replicated the "len" bug when I duplicated and 
modified the original code.  I will pull Wolfgang's fix tonight and see 
what needs to be done to make all three bugs go away.

WRT to building a multi-image, I'm looking to combine linux, a dtb, and 
an initrd into a single image to test that path of bootm (the path I had 
the above screwups in).

Timur's hint for me was:
> mkimage -A ppc -T flat_dt -C none -d mpc836x_mds.dtb mpc836x_mds.dtu
> 
> This, of course, creates a dtu with a value of 0 for ih_load.  You
> probably need to specify -a or -e to set that field.
but I don't understand how to build an image with all three pieces in it 
(I tried to use the ":" in the -d source parameter and mkimage just got 
confused, couldn't find the files).  Am I expecting too much???  Should 
I just be wrapping the three pieces individually and loading them 
separately?  What exactly are you doing to test this, Timur?

Best regards,
gvb