From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alison Chaiken <alison@peloton-tech.com>
Date: Mon, 12 Jun 2017 07:24:17 -0700
Subject: [U-Boot] [PATCH v6 3/3] GPT: provide commands to selectively
 rename partitions
In-Reply-To: <20170612074557.E32A3120432@gemini.denx.de>
References: <20170610065138.2410A120BB0@gemini.denx.de>
 <1497137617-772-1-git-send-email-alison@peloton-tech.com>
 <20170612074557.E32A3120432@gemini.denx.de>
Message-ID: <CAOuSAjdHerD7iWSwv5HQmx07nALRHschnH5=XToNEZDqA9JsvQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.


>
> > 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?

Thanks,
Alison Chaiken
Peloton Technology