From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Mon, 12 Jun 2017 10:56:46 -0400 Subject: [U-Boot] [PATCH v6 3/3] GPT: provide commands to selectively rename partitions In-Reply-To: References: <20170610065138.2410A120BB0@gemini.denx.de> <1497137617-772-1-git-send-email-alison@peloton-tech.com> <20170612074557.E32A3120432@gemini.denx.de> Message-ID: <20170612145646.GY10782@bill-the-cat> List-Id: 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 07:24:17AM -0700, Alison Chaiken wrote: > On Mon, Jun 12, 2017 at 12:45 AM, Wolfgang Denk 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: