[U-Boot] [PATCH v6 3/3] GPT: provide commands to selectively rename partitions

[Tom Rini <trini@konsulko.com>]

On Mon, Jun 12, 2017 at 07:24:17AM -0700, Alison Chaiken wrote:
> On Mon, Jun 12, 2017 at 12:45 AM, Wolfgang Denk <wd@denx.de> wrote:
> 
> > Dear Alison,
> >
> > In message <1497137617-772-1-git-send-email-alison@peloton-tech.com> you
> > wrote:
> > >
> > > This patch provides support in u-boot for renaming GPT
> > > partitions.  The renaming is accomplished via new 'gpt swap'
> > > and 'gpt rename' commands.
> >
> > Thanks.
> >
> > One question: can multiple GPT partitions have the same name?
> 
> The idea behind the 'swap' mode is that a storage device can have two
> sets of partitions, one set all named 'primary' and one set all named
> 'backup'.  The software updater in userspace can then simply rename
> the partitions with sgdisk in order to pick the new image.   The swap
> mode changes the whole set of labels at once, so there's little chance
> of being interrupted.

I'm a little confused here now then.  Can you provide an example set of
usage, in the cover letter for v8 of the series (see below) that makes
it a bit clearer?  Thanks!

> > > The 'swap' mode prints a warning if no matching partition names
> > > are found.  If only one matching name of a provided pair is found, it
> > > renames the matching partitions to the new name.
> >
> > I see a problem here...
> >
> > > +     if (!strcmp(subcomm, "swap")) {
> > > +             if ((strlen(name1) > PART_NAME_LEN) || (strlen(name2) >
> > PART_NAME_LEN)) {
> > > +                     printf("Names longer than %d characters are
> > truncated.\n", PART_NAME_LEN);
> > > +                     return -EINVAL;
> > > +             }
> > > +             list_for_each(pos, &disk_partitions) {
> > > +                     curr = list_entry(pos, struct disk_part, list);
> > > +                     if (!strcmp((char *)curr->gpt_part_info.name,
> > name1)) {
> > > +                             strcpy((char *)curr->gpt_part_info.name,
> > name2);
> > > +                             changed++;
> > > +                     }
> > > +                     else if (!strcmp((char *)curr->gpt_part_info.name,
> > name2)) {
> > > +                             strcpy((char *)curr->gpt_part_info.name,
> > name1);
> > > +                             changed++;
> > > +                     }
> > > +
> > > +             }
> > > +             if (changed == 0) {
> > > +                     printf("No matching partition names were
> > found.\n");
> > > +                     return ret;
> > > +             }
> >
> > You will never know if there really was a pair of names that was
> > swapped.  Just a single rename of name1->name2 _or_ of name2->name1
> > will make the user think everything was fine.
> >
> 
> Point taken.   The last version I posted has two counters instead of just
> changed in order to address this problem.
> 
> 
> 
> > [Note: I'm in the process of relocating and will be offline for the
> > next 2..3. days.  Don't expect more comments from mee soon.
> > Sorry...]
> > Best regards,
> > Wolfgang Denk
> 
> 
> 
> One additional note: the last version I posted worked fine for the sandbox,
> but wouldn't link for an ARM target with the Linaro toolchain, as the
> linker couldn't find atoi().   I guess the libc for the x86 compiler
> includes it.   To test on ARM, I copied in simple_atoi() from
> lib/vsprintf.c, but assuredly that is an ugly solution.    Does anyone have
> a better idea to solve this problem?

Looking at the man page for atoi:
The  atoi() function converts the initial portion of the string pointed
to by nptr to int.  The behavior is the same as

	strtol(nptr, NULL, 10);

So we should just re-work to use simple_strtol here.  And since you
brought it up on IRC, I suppose at this point a 'clean' posting of all 5
patches marked as 'v8' might make it easier to see what's what and in
what order.  Thanks again!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170612/a0d7e5bc/attachment.sig>