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

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

Information
tsoome
illumos-gate
8395
3b62b2f...
Reviewers
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




   

   







  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




jbk




   

   







  

 



 

  


   

   

    
 
jbk


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




rm




   

   







  

 



 

  


   

   

    
 
rm


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    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.
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 















 





 

  

   




jbk




   

   







  

 



 

  


   

   

    
 
jbk


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




rm




   

   







  

 



 

  


   

   

    
 
rm


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




marcel




   

   





  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    




    
    While touching this please add num_blocks here.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 

   
    
    
    




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

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 















 





 

  

   




marcel




   

   







  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




rm




   

   







  

 



 

  


   

   

    
 
rm


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




jbk




   

   







  

 



 

  


   

   

    
 
jbk


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...