From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Van Baren Date: Fri, 04 May 2007 10:22:09 -0400 Subject: [U-Boot-Users] [PATCH v3] Fix initrd length miscalculation in bootm command when using a dtu In-Reply-To: <200705041546.13452.sr@denx.de> References: <20070504082836.4C04C353AF1@atlas.denx.de> <463B3463.1030702@freescale.com> <200705041546.13452.sr@denx.de> Message-ID: <463B4191.1000706@smiths-aerospace.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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