9090 ses: using integer constants in boolean context, the expression will always evaluate to 'true'

Review Request #856 — Created Feb. 10, 2018 and submitted

tsoome
illumos-gate
9090
3c3146d...
general

In file included from ../../common/io/scsi/targets/ses.c:35:0:
../../common/io/scsi/targets/ses.c: In function 'ses_callback':
../../common/io/scsi/targets/ses.c:1329:37: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]
(err == EBUSY)? SES_BUSY_TIME: SES_RESTART_TIME,
../../common/sys/scsi/targets/ses.h:252:7: note: in definition of macro 'SES_ENABLE_RESTART'
(ms_time)? (drv_usectohz(ms_time * 1000)) : \
^~~
cc1: all warnings being treated as errors



  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
yuripv
  1. 
      
  2. May be move the logic here, and make it looks the same as the next line? Also, the argument to drv_usectohz is of the clock_t type.

    i.e.

    clock_t ms_time = err == EBUSY ? SES_BUSY_TIME : SES_RESTART_TIME;
    
    1. I would keep the assignment separate from declaration, but otherwise it is better yep.

  3. 
      
tsoome
yuripv
  1. Ship It!
  2. 
      
andy_js
  1. Looking at the code I think it's actually the SES_ENABLE_RESTART macro that needs to be fixed. This seems to just skirt around the issue.

    1. Fixed how? It can be called with mstime being 0.

    2. Ye, I had the same concern.

  2. 
      
andy_js
  1. Never mind.

  2. 
      
igork
  1. 
      
  2. why not move it before SES_ENABLE_RESTART() ?

    1. technically it does not matter where we set it. Logically it is good to keep blocks of declarations, initializations, updates etc. In this case, we declare the variable and then in the next block, we set the values (initial or computed). Incidentally we only initialize the ms_time, we do not update it later.

  3. 
      
citrus
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...