8839 madv: error: variable 'rc' set but not used

Review Request #765 — Created Nov. 21, 2017 and submitted

tsoome
illumos-gate
8839
8b6dcc6...
general
../common/madv.c: In function 'shmat':
../common/madv.c:681:6: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
  int rc;
      ^~
../common/madv.c: In function 'mmap':
../common/madv.c:734:6: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
  int rc;
      ^~
../common/madv.c: In function 'mmap64':
../common/madv.c:786:6: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
  int rc;
      ^~
cc1: all warnings being treated as errors


yuripv
  1. Ship It!
  2. 
      
jgmills
  1. Here's a better way to do it, although perhaps it's too complex, starting with shmat() on line 671:
    
    677:#ifdef MADVDEBUG
    678:	int rc;
    682:#endif
    
    Then surround the two statements that set `rc' with:
    
    #ifdef MADVDEBUG
    #endif
    
    And provide other statments that return `(void)' in the #else case.
    
    That should make sure that `rc' is only defined when MADVDEBUG is defined.
    
    By the way, the format string and following parameters in these lines do not agree:
    
    708:		MADVPRINT(4, (stderr, "shmctl rc %d errno %d\n",
    709:		    strerror(errno)));
    
    You can apply the same solution to the mmap() and mmap64() functions, although they are simpler
    1. I am not too sure if that #ifdef chain is better, but here it is now:)

    2. Both are probably OK, but the 1st has the advantage of being more likely to produce the same copmiled output (and not need other testing).

  2. 
      
tsoome
jgmills
  1. Looks okay to me now.

  2. 
      
yuripv
  1. Ship It!
  2. 
      
gwr
  1. 
      
  2. usr/src/lib/madv/common/madv.c (Diff revision 2)
     
     
    You could have just added an __unused
    here (and the other two places like it)
    and not needed the other changes.
    
    Might have been easier to prove that
    change would not affect the binary...
    1. Which was exactly the first change, as you can see from https://www.illumos.org/rb/r/765/diff/1/ :D

  3. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...