PR comments: logical changes

Author: src-up

src/clonesetup/clonesetup-generator.c

@@ -112,6 +112,9 @@ static int generate_clone_units(const char *clone_name, const char *source_dev,
 
         /* symlink unit file to enable it */
         dmname = strjoin("dev-mapper-", e, ".device");
+        if (!dmname)
+                return log_oom();
+
         r = generator_add_symlink(arg_dest, dmname, "requires", unit);
         if (r < 0)
                 return r;

src/clonesetup/clonesetup-ioctl.c

@@ -10,9 +10,13 @@
 #include "fd-util.h"
 #include "log.h"
 #include "sd-device.h"
-#include "stdio-util.h"          /* xsprintf() */
 #include "string-util.h"
 
+/* According to the Linux kernel sysfs ABI, the size of a block device reported in
+ * /sys/block/<disk>/size is always expressed in units of 512-byte sectors, regardless
+ * of the physical block size of the device. */
+#define SECTOR_SIZE 512
+
 /* Returns the size in bytes of the block device at dev_path.
  * Loading the dm-clone table needs the source device size in sectors; sysfs
  * reports size in 512-byte sectors. This reads sysfs and returns bytes so the
@@ -124,8 +128,8 @@ int dm_clone_create_device(
                 const char *dest_dev,
                 const char *metadata_dev) {
 
+        _cleanup_free_ char *target_params = NULL;
         uint64_t src_dev_size_sectors, src_dev_size;
-        char target_params[256];
         int r;
 
         assert(name);
@@ -137,13 +141,14 @@ int dm_clone_create_device(
         if (r < 0)
                 return r;
 
-        src_dev_size_sectors = src_dev_size / 512;
+        src_dev_size_sectors = src_dev_size / SECTOR_SIZE;
 
         /* dm-clone target params: <metadata_dev> <dest_dev> <source_dev> <region_size> <hydration_threshold> [options]
          *   8 = region size in sectors (4KB regions with 512-byte sectors)
          *   1 = hydration threshold (regions to hydrate per batch)
          *   no_hydration = don't start automatic background hydration */
-        xsprintf(target_params, "%s %s %s 8 1 no_hydration", metadata_dev, dest_dev, source_dev);
+        if (asprintf(&target_params, "%s %s %s 8 1 no_hydration", metadata_dev, dest_dev, source_dev) < 0)
+                return log_oom();
 
         r = dm_clone_create(name);
         if (r < 0)

src/clonesetup/clonesetup.c

@@ -36,6 +36,8 @@ static int clone_device(const char *clone_name, const char *source_dev, const ch
         if (!clone_dev_path)
                 return log_oom();
 
+        /* Check before calling the DM ioctl to give a cleaner error message;
+         * DM_DEV_CREATE would return EEXIST too, but with a less obvious message. */
         if (access(clone_dev_path, F_OK) >= 0)
                 return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Device '%s' already exists.", clone_dev_path);