On Sat, Mar 03, 2007 at 10:05:39PM -0800, Steve Langasek wrote:
A further concern on this patch: On Sat, Mar 03, 2007 at 02:47:04AM +0100, David Härdeman wrote:diff -ur ./parted-1.7.1.orig/libparted/labels/mac.c ./parted-1.7.1/libparted/labels/mac.c --- ./parted-1.7.1.orig/libparted/labels/mac.c 2006-05-25 19:28:55.000000000 +0200 +++ ./parted-1.7.1/libparted/labels/mac.c 2007-03-03 02:41:42.000000000 +0100 @@ -1260,19 +1260,23 @@ return 1;case PED_PARTITION_LVM:- mac_data->is_lvm = state; - if (state) + if (state) { strcpy (mac_data->system_name, "Linux_LVM"); - else - mac_partition_set_system (part, part->fs_type); + mac_data->is_lvm = state; + } else { + if (mac_data->is_lvm) + mac_partition_set_system (part, part->fs_type); + } return 1;So in this case, if (!state), mac_data->is_lvm is never un-set. Is that not an issue?
The "mac_data->is_lvm = state;" could probably be kept. I'm not sure why that isn't done in the current upstream version (it does the state change for the other flags). It needs to be moved to the last line though (just before return 1).
It's also interesting to note that some flags (e.g. PED_PARTITION_SWAP) clears other flags when set (PED_PARTITION_ROOT in that case) but there are still many nonsensical combinations that *are* allowed (PED_PARTITION_SWAP & PED_PARTITION_BOOT...eeh?)
Also, the mac_partition_set_system will change the system_name which in effect means that the other flags are no longer valid (did I mention that the mac partitioning code was confusing yet?).
In summary, I think the mac code needs some more work, and that the current patch + the mac_data->is_something flag clearing is the best we can do ATM.
-- David Härdeman