8457 libstmf: psStmfProp and psStmfPropVal may be used uninitialized

Review Request #614 — Created July 4, 2017 and submitted

tsoome
illumos-gate
8457
d1a91b8...
general
../common/store.c: In function 'iPsGetSetStmfProp':
../common/store.c:2397:6: error: 'psStmfProp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  if (scf_pg_get_property(pg, psStmfProp, prop) == -1) {
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../common/store.c:2447:7: error: 'psStmfPropVal' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   if (scf_value_set_astring(value, psStmfPropVal) == -1) {
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


  • 0
  • 0
  • 1
  • 1
  • 2
Description From Last Updated
igork
  1. Ship It!
  2. 
      
yuripv
  1. 
      
  2. usr/src/lib/libstmf/common/store.c (Diff revision 1)
     
     

    Not sure if these additional checks help anything, we have already checked that getSet is either GET or SET, and have a "default" entry for both cases.

  3. 
      
tsoome
yuripv
  1. Ship It!
  2. 
      
igork
  1. Ship It!
  2. 
      
tsoome
rm
  1. 
      
  2. usr/src/lib/libstmf/common/store.c (Diff revision 3)
     
     

    From looking more at the warning, I'm not sure that just initializing this is correct. While it does gag the warning, if we end up using the value of NULL in scf_value_set_astring() then we'll end up having a segmentation fault because we dereferenced NULL in strlcpy(). I think that the reason the analysis happens that it can be NULL is that we don't do anything if getSet is anything either than GET or SET. Perhaps if we changed this such that:

    if (getSet == GET) {
        ...
    } else if (getSet == SET) {
        ...
    } else {
        ret = STMF_PS_ERROR;
        goto out;
    }
    

    What do you think?

    1. See line 2299, we goto out if getSet is not GET or SET.

    2. Yep, the getSet value is verified early and is handled, so this scenario is elliminated already.

    3. Not sure how I missed that one, sorry. I guess in the case the root cause was an overly aggressive compiler?
    4. I also did miss it;) They also do admit the gcc 6 does have issues with false positives, but I also would really rather have some "unneeded" initializations than start to build version based suppressions in makefiles... I guess there also are some better approaches, but I think I'll leave an chance for other people to have some fun as well;)

  3. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...