Re: GPT name overflow

[Andreas Dilger]

On Jan 31, 2002  06:52 +1100, Andrew Clausen wrote:
> On Wed, Jan 30, 2002 at 11:52:53AM -0700, Andreas Dilger wrote:
> > Hmm, I was able (on subsequent tests) to get it to fail with "this is a long
> > name which h" but not "this is a long name which ".  This is with 1.5.6.  I
> > also tried with 1.6.0p2 with the same results.  Note that the name does not
> > even overflow the 36 (I think) character limit.
> 
> Both work here.  (1.6.0p2)

Hmm, maybe a glibc wchar problem?  I'm using glibc 2.1.3-33.  The message
appears to be from parted/strlist.c, and I hit it even with a totally
different string "trying a new string which is longer", but not the longer
"asdf asdf asdf asdf asdf asdf asdf asdf asfd".  Strange.

Hmm, I just tried to run "LANG=de parted /tmp/gpt" (also LANG=ca, LANG=pt)
and it replied:

Error during translation: Ungültiges oder unvollständiges Multi-Byte oder
Wide-Zeichen

which, as you can guess is the same as the "Error during translation:
Invalid or incomplete multibyte or wide character".  The two systems
I've tested this on are both based on an older TL 6.0 install, which
I've been updating as newer RPMs are needed.  Maybe it is a glibc bug.
The alternative is that there is a memory overrun somewhere which is
corrupting the wchar handling (no segfaults though), I don't know.

What libraries are involved with this translation?  Maybe I need to
update something?

> > > There might be two "options" that are the english and native
> > > versions of the same thing.  So, either we have to implement this
> > > at a higher level (PITA), or the UI code should know the difference
> > > between native and english words.  I think we need the later anyway.
> > 
> > I did this last night for the partition type (which is the one which I
> > see most when doing GPT partitions == always primary).  Patch below.
> 
> This is the "high-level" approach that I didn't like... 
> (low-level: one option [for anything]... don't bother asking]

Yes, that would be nicer, I agree, if it is reasonably easy to do.  It
turns out that my patch core dumped anyways.  One new bug that I see is
if you compile without fs support then do_mkpart() breaks because you
will never be able to find a matching fs type.  It should probably just
continue on (none of the lower-level code appears to require the fs_type.

> > Yes, having the low-level code also handle translations would be nice.
> > Maybe by keeping the translated word in the same option for display,
> > and the call to command_line_get_word() only returns the english word?
> > That would also simplify the resulting checks.
> 
> The checks are fairly easy already, via the str_list mess, hehe.
> But only displaying the translated words is better.  It's a bit
> silly displaying:
> 
> Erro: blah blah blah
> Yes/Sim/Ignore/Ignorar/Cancel/Cancelar

Well, for some classes of messages it may help to allow the user to see
both the english and translation, as I've heard it reported that sometimes
the english term is more widely used than a literally-translated word.

You could always re-structure the str_list to work like (vaguely):

        add_unique_str("primary", _("primary), list);

The (optionally) translated string is the only one which is displayed.
It's too bad that there isn't an inverse to the _() operation.  You would
get the return value from the selection the first string (== key), not the
translated (displayed) value.

> > I checked the GPT code, and it does appear to set the "flags" correctly.
> > Sadly, even though GPT uses 16 bytes for the UUID storage, you can only
> > set a single "flag" at one time (i.e. you can't have a boot RAID partition).
> > It must just not do the later comparison to display the flags.
> 
> Nope, more subtle: it's not updating the uuid, so it's not getting
> written to disk.  Parted always reads the partition table each
> time you type "print".

Well, I was looking at gpt_set_system() and not gpt_set_flags(), so you
are correct that it doesn't set the UUID at all.  Semi-tested patch below.
It removes the ->lvm, ->raid, ->boot booleans from the GPT-private data
and just compares the UUID fields directly.  This is better because you
avoid duplication of state, and it will also work for data read from disk.
It also adds support to the "swap" type which you didn't handle from
"set X swap on" before.


On a related note - how can you specify a particular partition type when
running mkpart for DOS partitions?  It seems parted (and libparted) make
this intentionally difficult to do.  What if you wanted to set a particular
GUID (unknown to parted) as the partition type for a GPT partition?

Cheers, Andreas
====================== parted-1.6.0-guid.diff ==============================
--- libparted/disk_gpt.c.orig   Wed Jan 23 11:27:15 2002
+++ libparted/disk_gpt.c        Wed Jan 30 14:29:35 2002
@@ -195,9 +195,6 @@
        efi_guid_t      type;
        efi_guid_t      uuid;
        char            name[37];
-       int             lvm;
-       int             raid;
-       int             boot;
 } GPTPartitionData;
 
 static PedDiskType gpt_disk_type;
@@ -838,9 +835,6 @@
                goto error_free_part;
 
        gpt_part_data->type = PARTITION_BASIC_DATA_GUID;
-       gpt_part_data->lvm = 0;
-       gpt_part_data->raid = 0;
-       gpt_part_data->boot = 0;
        uuid_generate ((char*) &gpt_part_data->uuid);
        swap_uuid_and_efi_guid((char*)(&gpt_part_data->uuid));
        strcpy (gpt_part_data->name, "");
@@ -903,32 +897,14 @@
 
        part->fs_type = fs_type;
 
-       if (gpt_part_data->lvm) {
-               gpt_part_data->type = PARTITION_LVM_GUID;
-               return 1;
-       }
-       if (gpt_part_data->raid) {
-               gpt_part_data->type = PARTITION_RAID_GUID;
-               return 1;
-       }
-       if (gpt_part_data->boot) {
-               gpt_part_data->type = PARTITION_SYSTEM_GUID;
-               return 1;
-       }
-
        if (fs_type) {
                if (strncmp (fs_type->name, "fat", 3) == 0
-                   || strcmp (fs_type->name, "ntfs") == 0) {
+                   || strcmp (fs_type->name, "ntfs") == 0)
                        gpt_part_data->type = PARTITION_MSFT_RESERVED_GUID;
-                       return 1;
-               }
-               if (strstr (fs_type->name, "swap")) {
+               else if (strstr (fs_type->name, "swap"))
                        gpt_part_data->type = PARTITION_SWAP_GUID;
-                       return 1;
-               }
        }
 
-       gpt_part_data->type = PARTITION_BASIC_DATA_GUID;
        return 1;
 }
 
@@ -994,21 +970,29 @@
 
        switch (flag) {
        case PED_PARTITION_BOOT:
-               gpt_part_data->boot = state;
                if (state)
-                       gpt_part_data->raid = gpt_part_data->lvm = 0;
+                       gpt_part_data->type = PARTITION_SYSTEM_GUID;
+               else if (!guid_cmp(gpt_part_data->type, PARTITION_SYSTEM_GUID))
+                       gpt_part_data->type = PARTITION_BASIC_DATA_GUID;
                break;
        case PED_PARTITION_RAID:
-               gpt_part_data->raid = state;
                if (state)
-                       gpt_part_data->boot = gpt_part_data->lvm = 0;
+                       gpt_part_data->type = PARTITION_RAID_GUID;
+               else if (!guid_cmp(gpt_part_data->type, PARTITION_RAID_GUID))
+                       gpt_part_data->type = PARTITION_BASIC_DATA_GUID;
                break;
        case PED_PARTITION_LVM:
-               gpt_part_data->lvm = state;
                if (state)
-                       gpt_part_data->boot = gpt_part_data->raid = 0;
+                       gpt_part_data->type = PARTITION_LVM_GUID;
+               else if (!guid_cmp(gpt_part_data->type, PARTITION_LVM_GUID))
+                       gpt_part_data->type = PARTITION_BASIC_DATA_GUID;
                break;
        case PED_PARTITION_SWAP:
+               if (state)
+                       gpt_part_data->type = PARTITION_SWAP_GUID;
+               else if (!guid_cmp(gpt_part_data->type, PARTITION_SWAP_GUID))
+                       gpt_part_data->type = PARTITION_BASIC_DATA_GUID;
+               break;
        case PED_PARTITION_ROOT:
        case PED_PARTITION_HIDDEN:
        case PED_PARTITION_LBA:
@@ -1027,12 +1011,13 @@
 
        switch (flag) {
        case PED_PARTITION_RAID:
-               return gpt_part_data->raid;
+               return !guid_cmp(gpt_part_data->type, PARTITION_RAID_GUID);
        case PED_PARTITION_LVM:
-               return gpt_part_data->lvm;
+               return !guid_cmp(gpt_part_data->type, PARTITION_LVM_GUID);
        case PED_PARTITION_BOOT:
-               return gpt_part_data->boot;
+               return !guid_cmp(gpt_part_data->type, PARTITION_SYSTEM_GUID);
        case PED_PARTITION_SWAP:
+               return !guid_cmp(gpt_part_data->type, PARTITION_SWAP_GUID);
        case PED_PARTITION_LBA:
        case PED_PARTITION_ROOT:
        case PED_PARTITION_HIDDEN:
@@ -1050,8 +1035,8 @@
        case PED_PARTITION_RAID:
        case PED_PARTITION_LVM:
        case PED_PARTITION_BOOT:
-               return 1;
        case PED_PARTITION_SWAP:
+               return 1;
        case PED_PARTITION_ROOT:
        case PED_PARTITION_HIDDEN:
        case PED_PARTITION_LBA:
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/