From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Herring Date: Tue, 18 Sep 2012 20:18:52 -0500 Subject: [U-Boot] [PATCH V3 2/8] disk: fix get_device_and_partition() bootable search In-Reply-To: <1348007874-20466-3-git-send-email-swarren@wwwdotorg.org> References: <1348007874-20466-1-git-send-email-swarren@wwwdotorg.org> <1348007874-20466-3-git-send-email-swarren@wwwdotorg.org> Message-ID: <50591D7C.7070200@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 09/18/2012 05:37 PM, Stephen Warren wrote: > From: Stephen Warren > > The existing get_device_and_partition() bootable search loop attempts > to save the current best known partition in variable best_part. However, > it's actually just saving a copy of the (static) "info" variable, and > hence ends up not doing anything much useful if no bootable partition > is found. Fix this by reworking the loop to: > > a) When reading information about a partition, always read into the > caller-supplied buffer; that way, if we find a bootable partition, > there's no need to copy any information. > b) Save the first known valid partition's structure content rather than > pointer in the search loop, and restore the structure content rather > than the pointer when the loop exits. > > Signed-off-by: Stephen Warren > --- > v3: New patch. You might as well squash these into my original commits. Rob > --- > disk/part.c | 38 +++++++++++++++++++++++++++++--------- > 1 files changed, 29 insertions(+), 9 deletions(-) > > diff --git a/disk/part.c b/disk/part.c > index 6caf6d2..277a243 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -447,7 +447,7 @@ int get_device_and_partition(const char *ifname, const char *dev_str, > int p; > int part = 0; > char *part_str; > - disk_partition_t *best_part = NULL; > + disk_partition_t tmpinfo; > > if (dev_str) > dev = simple_strtoul(dev_str, &ep, 16); > @@ -483,24 +483,44 @@ int get_device_and_partition(const char *ifname, const char *dev_str, > part = (int)simple_strtoul(++part_str, NULL, 16); > ret = get_partition_info(desc, part, info); > } else { > - /* find the first bootable partition. If none are bootable, > - * fall back to the first valid partition */ > + /* > + * Find the first bootable partition. > + * If none are bootable, fall back to the first valid partition. > + */ > for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) { > - ret = get_partition_info(desc, p, info); > + ret = get_partition_info(*dev_desc, p, info); > if (ret) > continue; > > - if (!best_part || info->bootable) { > - best_part = info; > + /* > + * First valid partition, or new better partition? > + * If so, save partition ID. > + */ > + if (!part || info->bootable) > part = p; > - } > > + /* Best possible partition? Stop searching. */ > if (info->bootable) > break; > + > + /* > + * We now need to search further for best possible. > + * If we what we just queried was the best so far, > + * save the info since we over-write it next loop. > + */ > + if (part == p) > + tmpinfo = *info; > } > - info = best_part; > - if (part) > + /* If we found any acceptable partition */ > + if (part) { > + /* > + * If we searched all possible partition IDs, > + * return the first valid partition we found. > + */ > + if (p == MAX_SEARCH_PARTITIONS + 1) > + *info = tmpinfo; > ret = 0; > + } > } > if (ret) { > printf("** Invalid partition %d, use `dev[:part]' **\n", part); >