Date: Sun, 12 Apr 2015 12:54:10 -0400 (EDT) From: Benjamin Kaduk <bjk@freebsd.org> To: Garrett Wollman <wollman@csail.MIT.EDU> Cc: freebsd-fs@freebsd.org Subject: Re: ZFS compiler warnings Message-ID: <alpine.GSO.1.10.1504121251110.22210@multics.mit.edu> In-Reply-To: <21800.25526.8680.435299@khavrinen.csail.mit.edu> References: <21800.25526.8680.435299@khavrinen.csail.mit.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 10 Apr 2015, Garrett Wollman wrote:
> 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.
n1256.pdf lists as a "common warning" (annex I) the case where a value is
assigned to a variable of enumerated type and the value is not from the
same set of enumerated constants, so any assignment of ZPROP_INVAL to a
variable of type zpool_prop_t should trigger a warning with the current
code. (If there are no such assignments, then why are we bothering to
check against it?)
> The following is a minimal change that fixes the warnings in the
> kernel alone (beware whitespace lossage) -- I haven't checked the
> userland side:
It seems like the right thing to do, to me, but I am not really a ZFS guy.
-Ben
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.GSO.1.10.1504121251110.22210>