GCC 7.1 fixes

GCC 7.1 with will warn when we're not checking the snprintf()
return code in cases where the buffer could be truncated. This
patch either checks the snprintf return code (where applicable),
or converts the snprintfs to a new SNPRINTF_TRUNC() macro,
which silently truncates without the compiler error.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>

cmd/ztest/ztest.c

@@ -126,6 +126,7 @@
 #include <sys/fs/zfs.h>
 #include <zfs_fletcher.h>
 #include <libnvpair.h>
+#include <zfs_comutil.h>
 #ifdef __GLIBC__
 #include <execinfo.h> /* for backtrace() */
 #endif
@@ -3540,8 +3541,9 @@ ztest_snapshot_destroy(char *osname, uint64_t id)
 	char snapname[ZFS_MAX_DATASET_NAME_LEN];
 	int error;
 
-	(void) snprintf(snapname, sizeof (snapname), "%s@%llu", osname,
-	    (u_longlong_t)id);
+	if (snprintf(snapname, sizeof (snapname), "%s@%llu", osname,
+	    (u_longlong_t)id) >= sizeof (snapname))
+		return (B_FALSE);
 
 	error = dsl_destroy_snapshot(snapname, B_FALSE);
 	if (error != 0 && error != ENOENT)
@@ -3565,8 +3567,8 @@ ztest_dmu_objset_create_destroy(ztest_ds_t *zd, uint64_t id)
 
 	(void) rw_rdlock(&ztest_name_lock);
 
-	(void) snprintf(name, sizeof (name), "%s/temp_%llu",
-	    ztest_opts.zo_pool, (u_longlong_t)id);
+	SNPRINTF_TRUNC(name, sizeof (name), "%s/temp_%llu", ztest_opts.zo_pool,
+	    (u_longlong_t)id);
 
 	/*
 	 * If this dataset exists from a previous run, process its replay log
@@ -3744,15 +3746,15 @@ ztest_dsl_dataset_promote_busy(ztest_ds_t *zd, uint64_t id)
 
 	ztest_dsl_dataset_cleanup(osname, id);
 
-	(void) snprintf(snap1name, ZFS_MAX_DATASET_NAME_LEN,
+	SNPRINTF_TRUNC(snap1name, ZFS_MAX_DATASET_NAME_LEN,
 	    "%s@s1_%llu", osname, (u_longlong_t)id);
-	(void) snprintf(clone1name, ZFS_MAX_DATASET_NAME_LEN,
+	SNPRINTF_TRUNC(clone1name, ZFS_MAX_DATASET_NAME_LEN,
 	    "%s/c1_%llu", osname, (u_longlong_t)id);
-	(void) snprintf(snap2name, ZFS_MAX_DATASET_NAME_LEN,
+	SNPRINTF_TRUNC(snap2name, ZFS_MAX_DATASET_NAME_LEN,
 	    "%s@s2_%llu", clone1name, (u_longlong_t)id);
-	(void) snprintf(clone2name, ZFS_MAX_DATASET_NAME_LEN,
+	SNPRINTF_TRUNC(clone2name, ZFS_MAX_DATASET_NAME_LEN,
 	    "%s/c2_%llu", osname, (u_longlong_t)id);
-	(void) snprintf(snap3name, ZFS_MAX_DATASET_NAME_LEN,
+	SNPRINTF_TRUNC(snap3name, ZFS_MAX_DATASET_NAME_LEN,
 	    "%s@s3_%llu", clone1name, (u_longlong_t)id);
 
 	error = dmu_objset_snapshot_one(osname, strchr(snap1name, '@') + 1);
@@ -6120,7 +6122,7 @@ ztest_thread(void *arg)
 static void
 ztest_dataset_name(char *dsname, char *pool, int d)
 {
-	(void) snprintf(dsname, ZFS_MAX_DATASET_NAME_LEN, "%s/ds_%d", pool, d);
+	SNPRINTF_TRUNC(dsname, ZFS_MAX_DATASET_NAME_LEN, "%s/ds_%d", pool, d);
 }
 
 static void
@@ -6420,7 +6422,7 @@ ztest_run(ztest_shared_t *zs)
 	 */
 	if (ztest_random(2) == 0) {
 		char name[ZFS_MAX_DATASET_NAME_LEN];
-		(void) snprintf(name, sizeof (name), "%s_import",
+		SNPRINTF_TRUNC(name, sizeof (name), "%s_import",
 		    ztest_opts.zo_pool);
 		ztest_spa_import_export(ztest_opts.zo_pool, name);
 		ztest_spa_import_export(name, ztest_opts.zo_pool);
@@ -7000,7 +7002,7 @@ main(int argc, char **argv)
 			char tmpname[ZFS_MAX_DATASET_NAME_LEN];
 			kernel_fini();
 			kernel_init(FREAD | FWRITE);
-			(void) snprintf(tmpname, sizeof (tmpname), "%s_tmp",
+			SNPRINTF_TRUNC(tmpname, sizeof (tmpname), "%s_tmp",
 			    ztest_opts.zo_pool);
 			(void) spa_rename(tmpname, ztest_opts.zo_pool);
 		}

include/zfs_comutil.h

@@ -41,6 +41,27 @@ extern int zfs_spa_version_map(int zpl_version);
 #define	ZFS_NUM_LEGACY_HISTORY_EVENTS 41
 extern const char *zfs_history_event_names[ZFS_NUM_LEGACY_HISTORY_EVENTS];
 
+/*
+ * Truncating snprintf() without compiler warnings
+ *
+ * GCC 7.1 with "-Wall" will warn in cases where we're not checking the
+ * snprintf() return code when the buffer could be truncated:
+ *
+ *     libzfs_iter.c: In function ‘zfs_iter_bookmarks’:
+ *     libzfs_iter.c:207:40: error: ‘snprintf’ output may be truncated
+ *     before the last format character [-Werror=format-truncation=]
+ *         (void) snprintf(name, sizeof (name), "%s#%s", zhp->zfs_name,
+ *
+ * In many cases we don't care and just want to silently truncate the buffer.
+ *
+ * This hack fools the compiler by making the size volatile, so that it isn't
+ * able to compute the truncation at compile time, and thus no error.
+ */
+#define	SNPRINTF_TRUNC(str, size, ...) do {				\
+	volatile int s = size;						\
+	snprintf(str, s, ##__VA_ARGS__);				\
+} while (0)
+
 #ifdef	__cplusplus
 }
 #endif

lib/libspl/include/assert.h

@@ -40,6 +40,24 @@ libspl_assert(const char *buf, const char *file, const char *func, int line)
 	abort();
 }
 
+/* printf version of libspl_assert */
+static inline int
+libspl_assertf(const char *file, const char *func, int line, char *format, ...)
+{
+	char *buf = NULL;
+	int size;
+	va_list args;
+	va_start(args, format);
+	size = vsnprintf(NULL, 0, format, args);
+	buf = alloca(size + 1);
+	if (buf == NULL)
+		buf = "<can't alloc>";
+	else
+		vsnprintf(buf, size, format, args);
+	va_end(args);
+	return (libspl_assert(buf, file, func, line));
+}
+
 #ifdef verify
 #undef verify
 #endif
@@ -55,13 +73,9 @@ libspl_assert(const char *buf, const char *file, const char *func, int line)
 do {									\
 	const TYPE __left = (TYPE)(LEFT);				\
 	const TYPE __right = (TYPE)(RIGHT);				\
-	if (!(__left OP __right)) {					\
-		char *__buf = alloca(256);				\
-		(void) snprintf(__buf, 256,				\
-		    "%s %s %s (0x%llx %s 0x%llx)", #LEFT, #OP, #RIGHT,	\
-		    (u_longlong_t)__left, #OP, (u_longlong_t)__right);	\
-		libspl_assert(__buf, __FILE__, __FUNCTION__, __LINE__);	\
-	}								\
+	if (!(__left OP __right))					\
+		libspl_assertf(__FILE__, __FUNCTION__, __LINE__,	\
+		    "%s %s %s (0x%llx %s 0x%llx)", #LEFT, #OP, #RIGHT);	\
 } while (0)
 
 #define	VERIFY3S(x, y, z)	VERIFY3_IMPL(x, y, z, int64_t)

lib/libzfs/libzfs_dataset.c

@@ -3566,8 +3566,9 @@ zfs_check_snap_cb(zfs_handle_t *zhp, void *arg)
 	char name[ZFS_MAX_DATASET_NAME_LEN];
 	int rv = 0;
 
-	(void) snprintf(name, sizeof (name),
-	    "%s@%s", zhp->zfs_name, dd->snapname);
+	if (snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name,
+	    dd->snapname) >= sizeof (name))
+		return (EINVAL);
 
 	if (lzc_exists(name))
 		verify(nvlist_add_boolean(dd->nvl, name) == 0);
@@ -3788,8 +3789,9 @@ zfs_snapshot_cb(zfs_handle_t *zhp, void *arg)
 	int rv = 0;
 
 	if (zfs_prop_get_int(zhp, ZFS_PROP_INCONSISTENT) == 0) {
-		(void) snprintf(name, sizeof (name),
-		    "%s@%s", zfs_get_name(zhp), sd->sd_snapname);
+		if (snprintf(name, sizeof (name), "%s@%s", zfs_get_name(zhp),
+		    sd->sd_snapname) >= sizeof (name))
+			return (EINVAL);
 
 		fnvlist_add_boolean(sd->sd_nvl, name);
 
@@ -4533,8 +4535,9 @@ zfs_hold_one(zfs_handle_t *zhp, void *arg)
 	char name[ZFS_MAX_DATASET_NAME_LEN];
 	int rv = 0;
 
-	(void) snprintf(name, sizeof (name),
-	    "%s@%s", zhp->zfs_name, ha->snapname);
+	if (snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name,
+	    ha->snapname) >= sizeof (name))
+		return (EINVAL);
 
 	if (lzc_exists(name))
 		fnvlist_add_string(ha->nvl, name, ha->tag);
@@ -4653,8 +4656,11 @@ zfs_release_one(zfs_handle_t *zhp, void *arg)
 	int rv = 0;
 	nvlist_t *existing_holds;
 
-	(void) snprintf(name, sizeof (name),
-	    "%s@%s", zhp->zfs_name, ha->snapname);
+	if (snprintf(name, sizeof (name), "%s@%s", zhp->zfs_name,
+	    ha->snapname) >= sizeof (name)) {
+		ha->error = EINVAL;
+		rv = EINVAL;
+	}
 
 	if (lzc_get_holds(name, &existing_holds) != 0) {
 		ha->error = ENOENT;

lib/libzfs/libzfs_iter.c

@@ -204,8 +204,11 @@ zfs_iter_bookmarks(zfs_handle_t *zhp, zfs_iter_f func, void *data)
 		bmark_name = nvpair_name(pair);
 		bmark_props = fnvpair_value_nvlist(pair);
 
-		(void) snprintf(name, sizeof (name), "%s#%s", zhp->zfs_name,
-		    bmark_name);
+		if (snprintf(name, sizeof (name), "%s#%s", zhp->zfs_name,
+		    bmark_name) >= sizeof (name)) {
+			err = EINVAL;
+			goto out;
+		}
 
 		nzhp = make_bookmark_handle(zhp, name, bmark_props);
 		if (nzhp == NULL)

lib/libzfs/libzfs_sendrecv.c

@@ -1867,9 +1867,13 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 			    drr_versioninfo, DMU_COMPOUNDSTREAM);
 			DMU_SET_FEATUREFLAGS(drr.drr_u.drr_begin.
 			    drr_versioninfo, featureflags);
-			(void) snprintf(drr.drr_u.drr_begin.drr_toname,
+			if (snprintf(drr.drr_u.drr_begin.drr_toname,
 			    sizeof (drr.drr_u.drr_begin.drr_toname),
-			    "%s@%s", zhp->zfs_name, tosnap);
+			    "%s@%s", zhp->zfs_name, tosnap) >=
+			    sizeof (drr.drr_u.drr_begin.drr_toname)) {
+				err = EINVAL;
+				goto stderr_out;
+			}
 			drr.drr_payloadlen = buflen;
 
 			err = dump_record(&drr, packbuf, buflen, &zc, outfd);

module/zfs/zfs_ioctl.c

@@ -3627,8 +3627,10 @@ zfs_ioc_destroy(zfs_cmd_t *zc)
 			 */
 			char namebuf[ZFS_MAX_DATASET_NAME_LEN + 6];
 
-			(void) snprintf(namebuf, sizeof (namebuf),
-			    "%s/%s", zc->zc_name, recv_clone_name);
+			if (snprintf(namebuf, sizeof (namebuf), "%s/%s",
+			    zc->zc_name, recv_clone_name) >=
+			    sizeof (namebuf))
+				return (EINVAL);
 
 			/*
 			 * Try to remove the hidden child (%recv) and after

module/zfs/zvol.c

@@ -79,6 +79,7 @@
 #include <sys/zfs_znode.h>
 #include <sys/spa_impl.h>
 #include <sys/zvol.h>
+#include <zfs_comutil.h>
 #include <linux/blkdev_compat.h>
 
 unsigned int zvol_inhibit_dev = 0;
@@ -2086,7 +2087,7 @@ zvol_rename_minors_impl(const char *oldname, const char *newname)
 		} else if (strncmp(zv->zv_name, oldname, oldnamelen) == 0 &&
 		    (zv->zv_name[oldnamelen] == '/' ||
 		    zv->zv_name[oldnamelen] == '@')) {
-			snprintf(name, MAXNAMELEN, "%s%c%s", newname,
+			SNPRINTF_TRUNC(name, MAXNAMELEN, "%s%c%s", newname,
 			    zv->zv_name[oldnamelen],
 			    zv->zv_name + oldnamelen + 1);
 			zvol_rename_minor(zv, name);