8607 zfs: variable set but not used

Reviews
Diff

- Download Diff

Information
Submitter:	Toomas Soome
Repository:	illumos-gate
Branch:
Bugs:	8607
Depends On:
Commit:	25dd566...
Reviewers
Groups:	general
People:

Description

../../common/fs/zfs/zfs_ioctl.c: In function 'zfs_ioc_send_space':
../../common/fs/zfs/zfs_ioctl.c:5578:12: error: variable 'embedok' set but not used [-Werror=unused-but-set-variable]
  boolean_t embedok;
            ^~~~~~~
../../common/fs/zfs/zfs_ioctl.c:5576:12: error: variable 'largeblockok' set but not used [-Werror=unused-but-set-variable]
  boolean_t largeblockok;
            ^~~~~~~~~~~~
../../../uts/common/fs/zfs/dsl_pool.c: In function 'dsl_pool_create':
../../../uts/common/fs/zfs/dsl_pool.c:390:12: error: variable 'os' set but not used [-Werror=unused-but-set-variable]
  objset_t *os;
            ^~
cc1: all warnings being treated as errors

Testing Done

Issues

0
4
0
All Issues: 4

Description	From	Last Updated
Given that it's only unused if !_KERNEL, should we make this conditional there rather than saying it's always unused as ...	Robert Mustacchi	Oct. 11, 2017, 4:46 p.m.
Given that these are set but not used, is there a reason we're doing these and the corresponding nvlist_exists calls? ...	Robert Mustacchi	Oct. 11, 2017, 4:37 p.m.
a better solution would probably be to move this, and the declaration of os, into the ifdef KERNEL	Matthew Ahrens	Oct. 11, 2017, 7:25 p.m.
let's just delete these, as Rob suggested.	Matthew Ahrens	Oct. 11, 2017, 5:08 p.m.

Ship it!

Ship It!

usr/src/uts/common/fs/zfs/dsl_pool.c (Diff revision 1)

Show all issues

Given that it's only unused if !_KERNEL, should we make this conditional there rather than saying it's always unused as its only unused for the userland bits?

Toomas Soome Oct. 11, 2017, 4:46 p.m.

I did add the #if block, but just for visual - the attribute unused is only an hint for compiler anyhow:

unused
This attribute, attached to a variable, means that the variable is meant to be possibly unused. GCC will not produce a warning for this variable.

usr/src/uts/common/fs/zfs/zfs_ioctl.c (Diff revision 1)

Show all issues

Given that these are set but not used, is there a reason we're doing these and the corresponding nvlist_exists calls? Is there a reason to even keep these here? Why not just delete them and the calls to nvlist_exists()?

Change Summary:

removed useless largeblockok/embedok, fixed the comment and #if for os __unused

Commit:

-	11788b97870728635a4a9a905bc3ee502968c614
+	45e8e2b03da15014cb6f5e57bf40e2b0ab70aabb

Diff:

Revision 2 (+7 -7)

Show changes

	usr/src/uts/common/fs/zfs/dsl_pool.c
	usr/src/uts/common/fs/zfs/zfs_ioctl.c

usr/src/uts/common/fs/zfs/dsl_pool.c (Diff revision 1)

Show all issues

a better solution would probably be to move this, and the declaration of os, into the ifdef KERNEL

Toomas Soome Oct. 11, 2017, 7:25 p.m.

ok, done. I have to admit that I know next to nothing about ztest (which seems to be the only consumer for this code path), but running ztest without the arguments seems to behave.... except it seems to have crapload of bugs on its own (starting with dumping core when you have no ztest binary in /usr/bin and not cleaning up the /tmp).

usr/src/uts/common/fs/zfs/zfs_ioctl.c (Diff revision 1)