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

[Jim Meyering]

Phillip Susi wrote:

> The device-mapper partition sync code was still using the remove all
> partitions, then add new partitions method.  Refactor to use the same
> algorithm as regular disks: try to remove all, and ignore any that could
> not be removed but have not changed.
...
> diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
> index e2c4139..496e1c5 100644
> --- a/libparted/arch/linux.c
> +++ b/libparted/arch/linux.c
> @@ -2225,28 +2225,57 @@ zasprintf (const char *format, ...)
>    return r < 0 ? NULL : resultp;
>  }
>
> +static char *dm_canonical_path (PedDevice *dev)
> +{
> +        char*           dev_name = NULL;
> +        LinuxSpecific*  arch_specific = LINUX_SPECIFIC (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;
> +        dev_name = ped_malloc (strlen (dm_task_get_name (task)) +
> +                               strlen ("/dev/mapper/") + 1);
> +        strcpy (dev_name, "/dev/mapper/");
> +        strcat (dev_name, dm_task_get_name (task));
> +        /* Caution: dm_task_destroy frees dev_name.  */
> +        dm_task_destroy (task);
> +        return dev_name;
> +err:
> +        return NULL;
> +}

Another change:

I'm adjusting the function definition to put the name in column 1.
More importantly, please make a point not to use the combination
of malloc and str* functions when you can use something like
asprintf instead.  The latter is a lot more readable, and especially,
more maintainable.

Also fixed indentation in unrelated code.

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index c0aee14..b3909a0 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2225,9 +2225,9 @@ zasprintf (const char *format, ...)
   return r < 0 ? NULL : resultp;
 }

-static char *dm_canonical_path (PedDevice *dev)
+static char *
+dm_canonical_path (PedDevice *dev)
 {
-        char*           dev_name = NULL;
         LinuxSpecific*  arch_specific = LINUX_SPECIFIC (dev);

         /* Get map name from devicemapper */
@@ -2239,11 +2239,9 @@ static char *dm_canonical_path (PedDevice *dev)
                 goto err;
         if (!dm_task_run(task))
                 goto err;
-        dev_name = ped_malloc (strlen (dm_task_get_name (task)) +
-                               strlen ("/dev/mapper/") + 1);
-        strcpy (dev_name, "/dev/mapper/");
-        strcat (dev_name, dm_task_get_name (task));
-        /* Caution: dm_task_destroy frees dev_name.  */
+        char *dev_name = zasprintf ("/dev/mapper/%s", dm_task_get_name (task));
+        if (dev_name == NULL)
+                goto err;
         dm_task_destroy (task);
         return dev_name;
 err:
@@ -2655,8 +2653,8 @@ _dm_add_partition (PedDisk* disk, const PedPartition* part)
                               dev_name,
                               isdigit (dev_name[name_len - 1]) ? "p" : "",
                               part->num);
-           if (vol_name == NULL)
-                   goto err;
+        if (vol_name == NULL)
+                goto err;

         if (isdigit (dev_name[strlen (dev_name) - 1])) {
                 if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))