[parted-devel] [PATCH 1/6] libparted: refactor device-mapper partition sync code

[Jim Meyering]

Jim Meyering wrote:

> Jim Meyering wrote:
>> Phillip Susi wrote:
> ...
>> ...
>>> +static int
>>> +_dm_add_partition (PedDisk* disk, const PedPartition* part)
>>> +{
>>> +        LinuxSpecific*  arch_specific = LINUX_SPECIFIC (disk->dev);
>>> +
>>> +        /* Get map name from devicemapper */
>>> +        struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
>>> +        if (!task)
>>> +                goto err;
>>> +
>>> +        if (!dm_task_set_major_minor (task, arch_specific->major,
>>> +                                      arch_specific->minor, 0))
>>> +                goto err;
>>> +
>>> +        if (!dm_task_run(task))
>>> +                goto err;
>>> +
>>> +        const char *dev_name = dm_task_get_name (task);
>>> +        char *vol_name;
>>> +        if (isdigit (dev_name[strlen (dev_name) - 1])) {
>>> +                if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))
>>> +                        goto err;
>>> +        } else if ( ! (vol_name = zasprintf ("%s%d", dev_name, part->num)))
>>> +                goto err;
>>
>> Please combine those:
>>
>>            char *vol_name
>>              = zasprintf ("%s%s%d",
>>                           dev_name,
>>                           isdigit (dev_name[strlen (dev_name) - 1]) ? "p" : "",
>>                           part->num);
>>            if (vol_name == NULL)
>>                    goto err;
>
> Here's a 2nd amendment:
>
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index 22556e8..c0aee14 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2618,7 +2618,7 @@ _dm_get_partition_start_and_length(PedPartition const *part,
>
>          int major, minor;
>          char *params;
> -	char *target_type;
> +        char *target_type;
>          dm_get_next_target(task, NULL, (uint64_t *)start, (uint64_t *)length, &target_type, &params);
>          if (sscanf (params, "%d:%d %Ld", &major, &minor, start) != 3)
>                  goto err;
> @@ -2650,6 +2650,14 @@ _dm_add_partition (PedDisk* disk, const PedPartition* part)
>                  goto err;
>
>          const char *dev_name = dm_task_get_name (task);
> +        size_t name_len = strlen (dev_name);
> +        vol_name = zasprintf ("%s%s%d",
> +                              dev_name,
> +                              isdigit (dev_name[name_len - 1]) ? "p" : "",
> +                              part->num);
> +           if (vol_name == NULL)
> +                   goto err;
> +
>          if (isdigit (dev_name[strlen (dev_name) - 1])) {
>                  if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))
>                          goto err;

Well, obviously when I rewrote that, I intended to remove the old code,
not just insert the new.  Here's the removal half, along with
other changes:

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 0f59a7e..70b26a9 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -285,7 +285,7 @@ struct blkdev_ioctl_param {
 /* Maximum number of partitions supported by linux. */
 #define MAX_NUM_PARTS		64

-static char* _device_get_part_path (PedDevice* dev, int num);
+static char* _device_get_part_path (PedDevice const *dev, int num);
 static int _partition_is_mounted_by_path (const char* path);

 static int
@@ -2249,13 +2249,11 @@ err:
 }

 static char*
-_device_get_part_path (PedDevice *dev, int num)
+_device_get_part_path (PedDevice const *dev, int num)
 {
-        char *devpath;
-        if (dev->type == PED_DEVICE_DM)
-                devpath = dm_canonical_path (dev);
-        else devpath = dev->path;
-        int path_len = strlen (devpath);
+        char *devpath = (dev->type == PED_DEVICE_DM
+                         ? dm_canonical_path (dev) : dev->path);
+        size_t path_len = strlen (devpath);
         char *result;
         /* Check for devfs-style /disc => /partN transformation
            unconditionally; the system might be using udev with devfs rules,
@@ -2656,12 +2654,6 @@ _dm_add_partition (PedDisk* disk, const PedPartition* part)
         if (vol_name == NULL)
                 goto err;

-        if (isdigit (dev_name[strlen (dev_name) - 1])) {
-                if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))
-                        goto err;
-        } else if ( ! (vol_name = zasprintf ("%s%d", dev_name, part->num)))
-                goto err;
-
         /* Caution: dm_task_destroy frees dev_name.  */
         dm_task_destroy (task);
         task = NULL;