10314 nvmeadm: NULL pointer errors

Review Request #1407 — Created Jan. 28, 2019 and submitted

tsoome
illumos-gate
10314
cad0943...
general
nvmeadm_dev.c: In function 'nvme_ioctl':
nvmeadm_dev.c:44:53: error: comparison between pointer and integer [-Werror]
   if ((nioc.n_buf = (uintptr_t)calloc(*bufsize, 1)) == NULL)
                                                     ^~
cc1: all warnings being treated as errors


  • 0
  • 0
  • 0
  • 1
  • 1
Description From Last Updated
andy_js
  1. Ship It!
  2. 
      
rm
  1. Ship It!
  2. 
      
domag02
  1. 
      
  2. usr/src/cmd/nvmeadm/nvmeadm_dev.c (Diff revision 1)
     
     

    Drop this issue, if you think it's not important.
    It seems like calloc parameters are swapped over.

    void *
    calloc(size_t nelem, size_t elsize);
    

    Should be:

            if ((nioc.n_buf = (uintptr_t)calloc(1, *bufsize)) == 0)
    
    1. Yes, I did notice it but, it has to go into separate issue, so we can actually track the changes.

  3. 
      
tsoome
hans
  1. 
      
  2. usr/src/cmd/nvmeadm/nvmeadm_dev.c (Diff revision 1)
     
     

    Just checked the C standard (C99 and C11), and it seems NULL is still defined as "implementation defined null pointer". Has that changed recently?

    If not, I'd probably prefer to split this up, i.e allocate first and check for NULL, then assign it to that uintptr_t field for the ioctl.

    1. Of course NULL is implementation defined, unfortunately our implementation is with two faces - pointer and integer, and the result is total mess. uintptr_t is int type, however, so if we do decice to compare with nioc.n_buf, we better use explicit 0. IMO it is totally OK to check against uintptr_t and 0.

    2. This is an interesting one that made me do some further research. It's pretty abstract but it seems that for correctness, either the right hand side should be (uintptr_t)NULL or just split it into two lines and check if calloc is returning NULL before casting the return value and assigning to ncioc.n_buf in a separate step.

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

Status: Closed (submitted)

Loading...