Date: Fri, 10 Apr 2015 19:58:46 -0400 From: Garrett Wollman <wollman@csail.mit.edu> To: freebsd-fs@freebsd.org Subject: ZFS compiler warnings Message-ID: <21800.25526.8680.435299@khavrinen.csail.mit.edu>
next in thread | raw e-mail | index | archive | help
I notice on my 10.1 kernel builds warnings like the following:
/usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:2855:12: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always true [-Wtautological-constant-out-of-range-compare]
if (prop != ZPROP_INVAL && !zfs_prop_inheritable(prop))
~~~~ ^ ~~~~~~~~~~~
/usr/src/sys/modules/zfs/../../cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c:3810:11: warning: comparison of constant -1 with expression of type 'zfs_prop_t' is always false [-Wtautological-constant-out-of-range-compare]
if (prop == ZPROP_INVAL) {
~~~~ ^ ~~~~~~~~~~~
The warnings indicate that these checks are being optimized away by
clang. The code in head is the same. Does anyone have an idea of how
this should be fixed, in a way that would be acceptable upstream? (Do
we have someone who is the "official" liaison with the OpenZFS
project?)
My guess is that the definition of the zfs_prop_t enum itself needs to
include these two constants explicitly -- but that also has the effect
of changing the type of ZPROP_INVAL, which then means that
zpool_prop_t needs to have its own version of "invalid property"
defined, and that then requires cascading changes in a bunch of other
places.
The following is a minimal change that fixes the warnings in the
kernel alone (beware whitespace lossage) -- I haven't checked the
userland side:
Index: sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c
===================================================================
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c (revision 281265)
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa.c (working copy)
@@ -332,7 +332,7 @@
zprop_source_t src = ZPROP_SRC_DEFAULT;
zpool_prop_t prop;
- if ((prop = zpool_name_to_prop(za.za_name)) == ZPROP_INVAL)
+ if ((prop = zpool_name_to_prop(za.za_name)) == ZPOOL_PROP_INVAL)
continue;
switch (za.za_integer_length) {
@@ -422,7 +422,7 @@
zpool_prop_t prop = zpool_name_to_prop(propname);
switch (prop) {
- case ZPROP_INVAL:
+ case ZPOOL_PROP_INVAL:
if (!zpool_prop_feature(propname)) {
error = SET_ERROR(EINVAL);
break;
@@ -662,7 +662,7 @@
prop == ZPOOL_PROP_READONLY)
continue;
- if (prop == ZPOOL_PROP_VERSION || prop == ZPROP_INVAL) {
+ if (prop == ZPOOL_PROP_VERSION || prop == ZPOOL_PROP_INVAL) {
uint64_t ver;
if (prop == ZPOOL_PROP_VERSION) {
@@ -6308,7 +6308,7 @@
spa_feature_t fid;
switch (prop = zpool_name_to_prop(nvpair_name(elem))) {
- case ZPROP_INVAL:
+ case ZPOOL_PROP_INVAL:
/*
* We checked this earlier in spa_prop_validate().
*/
Index: sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h
===================================================================
--- sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h (revision 281265)
+++ sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h (working copy)
@@ -152,7 +152,9 @@
ZFS_PROP_SNAPSHOT_COUNT,
ZFS_PROP_REDUNDANT_METADATA,
ZFS_PROP_PREV_SNAP,
- ZFS_NUM_PROPS
+ ZFS_NUM_PROPS,
+ ZPROP_INVAL = -1,
+ ZPROP_CONT = -2
} zfs_prop_t;
typedef enum {
@@ -196,14 +198,18 @@
ZPOOL_PROP_FREEING,
ZPOOL_PROP_FRAGMENTATION,
ZPOOL_PROP_LEAKED,
- ZPOOL_NUM_PROPS
+ ZPOOL_NUM_PROPS,
+ ZPOOL_PROP_INVAL = -1,
+ ZPOOL_PROP_CONT = -2
} zpool_prop_t;
/* Small enough to not hog a whole line of printout in zpool(1M). */
#define ZPROP_MAX_COMMENT 32
+/*
#define ZPROP_CONT -2
#define ZPROP_INVAL -1
+*/
#define ZPROP_VALUE "value"
#define ZPROP_SOURCE "source"
There is also one spot in zio_checksum.c that has a similar problem,
but in this case the issue is that it assumes that an out-of-range
value can be bitwise-or'ed with a small enum. (The C standard allows
the compiler to use an eight-bit integer type as the underlying type
of 'enum zio_checksum', so to bitwise-or it with ZIO_CHECKSUM_VERIFY,
defined to be 256, is undefined.) A similar change eliminates this
warning. (The effect in both cases is to convince the compiler to
model the enum with a larger underlying type.)
-GAWollman
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?21800.25526.8680.435299>