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

Tom Rini
2017-06-12T14:56:46Z

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>