8395 mr_sas: sizeof on array function parameter will return size of pointer

Review Request #584 — Created June 15, 2017 and submitted

tsoome
illumos-gate
8395
3b62b2f...
general
../../common/io/mr_sas/mr_sas_tbolt.c:3012:21: error: 'sizeof' on array function parameter 'cdb' will return size of 'uint8_t * {aka unsigned char *}' [-Werror=sizeof-array-argument]
   bzero(cdb, sizeof (cdb));
                     ^
../../common/io/mr_sas/mr_sas_tbolt.c:2987:27: note: declared here
 mrsas_tbolt_set_pd_lba(U8 cdb[], uint8_t *cdb_len_ptr, U64 start_blk,
                           ^~~
../../common/io/mr_sas/mr_sas_tbolt.c:3045:21: error: 'sizeof' on array function parameter 'cdb' will return size of 'uint8_t * {aka unsigned char *}' [-Werror=sizeof-array-argument]
   bzero(cdb, sizeof (cdb));
                     ^
../../common/io/mr_sas/mr_sas_tbolt.c:2987:27: note: declared here
 mrsas_tbolt_set_pd_lba(U8 cdb[], uint8_t *cdb_len_ptr, U64 start_blk,
                           ^~~
../../common/io/mr_sas/mr_sas_tbolt.c:3065:21: error: 'sizeof' on array function parameter 'cdb' will return size of 'uint8_t * {aka unsigned char *}' [-Werror=sizeof-array-argument]
   bzero(cdb, sizeof (cdb));
                     ^
../../common/io/mr_sas/mr_sas_tbolt.c:2987:27: note: declared here
 mrsas_tbolt_set_pd_lba(U8 cdb[], uint8_t *cdb_len_ptr, U64 start_blk,
                           ^~~
cc1: all warnings being treated as errors


  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
marcel
  1. Ship It!
  2. 
      
jbk
  1. Ship It!
  2. 
      
rm
  1. 
      
  2. So, here and in the other places, I think we want to semantically zero the entire buffer which is a 32-byte (or maybe larger) array. I think we should probably change this so the size of the actual cdb entry is passed in.

    Alternatively we should use the length that was given to us that had the actual cdb length and zero all of that. This may be the better thing. Basically use cdb_len before we update it as that should represent the amount of data present.

    1. Well, the idea of using cdb_len did occur to me, but there is an small issue - for large LBA and cdb_len < 16, we do convert 6/10/12 CDB to 16 byte CDB - so we can not really use cdb_len.

      We can not assume anything else about the cdb size, because we do not know it. From code we can track that perhaps it is 32-bytes, but IMO that would be really dangerous thing to do. Considering that the current code does zero 4 or 8 bytes.

      And finally there is third option - the mrsas_tbolt_set_pd_lba() is local function, we can just update it to pass the CDB storage size. I think this would be the cleanest option anyhow, but I do not know about mr_sas background.

  3. 
      
tsoome
jbk
  1. Ship It!
  2. 
      
rm
  1. Ship It!
  2. 
      
marcel
  1. 
      
  2. While touching this please add num_blocks here.

  3. Since you changed the prototype above to U8 *, please change it here too.

  4. 
      
tsoome
marcel
  1. Ship It!
  2. 
      
rm
  1. Ship It!
  2. 
      
jbk
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...