8607 zfs: variable set but not used

Review Request #666 — Created Oct. 3, 2017 and submitted

tsoome
illumos-gate
8607
25dd566...
general
../../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


  • 0
  • 0
  • 4
  • 0
  • 4
Description From Last Updated
yuripv
  1. Ship It!
  2. 
      
rm
  1. 
      
  2. usr/src/uts/common/fs/zfs/dsl_pool.c (Diff revision 1)
     
     
    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?
    1. 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.
      
  3. usr/src/uts/common/fs/zfs/zfs_ioctl.c (Diff revision 1)
     
     
     
     
     
    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()?
  4. 
      
tsoome
mahrens
  1. 
      
  2. usr/src/uts/common/fs/zfs/dsl_pool.c (Diff revision 1)
     
     
     
     
     

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

    1. 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).

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

    let's just delete these, as Rob suggested.

  4. 
      
tsoome
mahrens
  1. Ship It!
  2. usr/src/uts/common/fs/zfs/dsl_pool.c (Diff revision 3)
     
     
     
     

    [nit] you could move the declaration down into the ifdef kernel block on line 444.

  3. 
      
tsoome
yuripv
  1. If it doesn't break ztest, ship it.

  2. 
      
igork
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...