libmultipath: fix -Wsign-compare warnings with snprintf()
authorMartin Wilck <mwilck@suse.com>
Thu, 7 Nov 2019 09:27:41 +0000 (09:27 +0000)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Mon, 2 Mar 2020 08:57:32 +0000 (09:57 +0100)
snprintf() returns int, but the size argument "n" is size_t.
Use safe_snprintf() to avoid -Wsign-compare warnings. At the same
time, improve these macros to check for errors in snprintf(), too.

Note: there are more uses of snprintf() in our code that may
need review, too. For now, I'm fixing only those that were causing
warnings.

Signed-off-by: Martin Wilck <mwilck@suse.com>
kpartx/devmapper.c
kpartx/kpartx.h
libmultipath/foreign/nvme.c
libmultipath/sysfs.c
libmultipath/util.c
libmultipath/util.h
libmultipath/wwids.c
multipath/main.c

index 97318c4..86731ea 100644 (file)
@@ -10,6 +10,7 @@
 #include <errno.h>
 #include <sys/sysmacros.h>
 #include "devmapper.h"
+#include "kpartx.h"
 
 #define _UUID_PREFIX "part"
 #define UUID_PREFIX _UUID_PREFIX "%d-"
@@ -107,7 +108,7 @@ strip_slash (char * device)
 static int format_partname(char *buf, size_t bufsiz,
                           const char *mapname, const char *delim, int part)
 {
-       if (snprintf(buf, bufsiz, "%s%s%d", mapname, delim, part) >= bufsiz)
+       if (safe_snprintf(buf, bufsiz, "%s%s%d", mapname, delim, part))
                return 0;
        strip_slash(buf);
        return 1;
index b20af1d..67edeb8 100644 (file)
 #define likely(x)       __builtin_expect(!!(x), 1)
 #define unlikely(x)     __builtin_expect(!!(x), 0)
 
+#define safe_snprintf(var, size, format, args...)                      \
+       ({                                                              \
+               size_t __size = size;                                   \
+               int __ret;                                              \
+                                                                       \
+               __ret = snprintf(var, __size, format, ##args);          \
+               __ret < 0 || (size_t)__ret >= __size;                   \
+       })
+
 #define safe_sprintf(var, format, args...)     \
-       snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
+               safe_snprintf(var, sizeof(var), format, ##args)
 
 #ifndef BLKSSZGET
 #define BLKSSZGET  _IO(0x12,104)       /* get block device sector size */
index e8ca516..09cdddf 100644 (file)
@@ -591,8 +591,7 @@ static void test_ana_support(struct nvme_map *map, struct udev_device *ctl)
                return;
 
        dev_t = udev_device_get_sysattr_value(ctl, "dev");
-       if (snprintf(sys_path, sizeof(sys_path), "/dev/char/%s", dev_t)
-           >= sizeof(sys_path))
+       if (safe_sprintf(sys_path, "/dev/char/%s", dev_t))
                return;
 
        fd = open(sys_path, O_RDONLY);
@@ -663,8 +662,7 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map)
                char *fn = di[i]->d_name;
                struct udev_device *ctrl, *udev;
 
-               if (snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s", fn)
-                   >= sizeof(pathbuf) - n)
+               if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n, "/%s", fn))
                        continue;
                if (realpath(pathbuf, realbuf) == NULL) {
                        condlog(3, "%s: %s: realpath: %s", __func__, THIS,
index 923b529..62ec2ed 100644 (file)
@@ -306,7 +306,7 @@ bool sysfs_is_multipathed(const struct path *pp)
        n = snprintf(pathbuf, sizeof(pathbuf), "/sys/block/%s/holders",
                     pp->dev);
 
-       if (n >= sizeof(pathbuf)) {
+       if (n < 0 || (size_t)n >= sizeof(pathbuf)) {
                condlog(1, "%s: pathname overflow", __func__);
                return false;
        }
@@ -327,9 +327,8 @@ bool sysfs_is_multipathed(const struct path *pp)
                int nr;
                char uuid[6];
 
-               if (snprintf(pathbuf + n, sizeof(pathbuf) - n,
-                            "/%s/dm/uuid", di[i]->d_name)
-                   >= sizeof(pathbuf) - n)
+               if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n,
+                                 "/%s/dm/uuid", di[i]->d_name))
                        continue;
 
                fd = open(pathbuf, O_RDONLY);
index ccc0de2..51c38c8 100644 (file)
@@ -212,8 +212,7 @@ int devt2devname(char *devname, int devname_len, char *devt)
                        continue;
 
                if ((major == tmpmaj) && (minor == tmpmin)) {
-                       if (snprintf(block_path, sizeof(block_path),
-                                    "/sys/block/%s", dev) >= sizeof(block_path)) {
+                       if (safe_sprintf(block_path, "/sys/block/%s", dev)) {
                                condlog(0, "device name %s is too long", dev);
                                fclose(fd);
                                return 1;
index 913ab7c..56bd78c 100644 (file)
@@ -29,9 +29,16 @@ void set_max_fds(rlim_t max_fds);
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
 
 #define safe_sprintf(var, format, args...)     \
-       snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
+       safe_snprintf(var, sizeof(var), format, ##args)
+
 #define safe_snprintf(var, size, format, args...)      \
-       snprintf(var, size, format, ##args) >= size
+       ({                                                              \
+               size_t __size = size;                                   \
+               int __ret;                                              \
+                                                                       \
+               __ret = snprintf(var, __size, format, ##args);          \
+               __ret < 0 || (size_t)__ret >= __size;                   \
+       })
 
 #define pthread_cleanup_push_cast(f, arg)              \
        pthread_cleanup_push(((void (*)(void *))&f), (arg))
index 291db8f..28a2150 100644 (file)
@@ -393,8 +393,7 @@ static int _failed_wwid_op(const char *wwid, bool rw,
        long lockfd;
        int r = -1;
 
-       if (snprintf(path, sizeof(path), "%s/%s", shm_dir, wwid)
-           >= sizeof(path)) {
+       if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
                condlog(1, "%s: path name overflow", __func__);
                return -1;
        }
index 347a440..1500775 100644 (file)
@@ -423,8 +423,7 @@ static int find_multipaths_check_timeout(const struct path *pp, long tmo,
 
        clock_gettime(CLOCK_REALTIME, &now);
 
-       if (snprintf(path, sizeof(path), "%s/%s", shm_find_mp_dir, pp->dev_t)
-           >= sizeof(path)) {
+       if (safe_sprintf(path, "%s/%s", shm_find_mp_dir, pp->dev_t)) {
                condlog(1, "%s: path name overflow", __func__);
                return FIND_MULTIPATHS_ERROR;
        }