8248 dboot: dboot_startkern.c should be more consistent about type casts

Review Request #523 — Created May 18, 2017 and submitted

tsoome
illumos-gate
8248
9d50a8a...
general
8248 dboot: dboot_startkern.c should be more consistent about type casts

For test, I did set up the sys/null.h defining NULL as (void *)0 for _BOOT bits.

  • 0
  • 0
  • 0
  • 2
  • 2
Description From Last Updated
andy_js
  1. Is casting NULL really necessary?

    1. the issue is that in our current code on gate, the NULL is 0, thats integer, not pointer and the build does not complain about it. Now if the NULL is actually a pointer ((void *)0), then the build will be broken (assigning pointer to unsigned int without the cast). So for correctness, yes, we really should cast it same as all other pointers.

  2. 
      
tsoome
tsoome
gwr
  1. 
      
  2. Since we know bm_hash is an int32 or int64, I'd find it less "noisy" to just write the contant zero instead of this cast. (Same for the other NULL assignments and/or comparisons.)

    1. well, the "nativeptr_t" is there to remind us it is pointer there. And mixing ints with pointers will end up in angry lint and compilers. Frankly, Im not interested about trying to figure out which cast there is really needed and which is not (if it was needed for studio and not needed for gcc) - this work is to make sure the same approach is used through the file. Especially considering how the NULL is defined in current kernel.

  3. 
      
gwr
  1. 
      
  2. Do you actually need the (native_ptr_t) part of that double cast? If the left side is int32 or int64 (depending on BOOT_TARGET...) then I would have thought just the (uintptr_t) cast would be sufficient. no?

    1. Same reasoning as above. Just keep the same logic on handling those assignments and casts, make sure we have clean build for now and once we have sane NULL, we can return and see what and why.

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

Status: Closed (submitted)

Loading...