10778 mac: NULL pointer errors

Review Request #1743 — Created April 14, 2019 and submitted

tsoome
illumos-gate
10778
0a4ee87...
general
../../common/io/mac/mac_hio.c: In function 'i_mac_share_free':
../../common/io/mac/mac_hio.c:97:22: error: comparison between pointer and integer [-Werror]
  if (mcip->mci_share == NULL) {
                      ^~
../../common/io/mac/mac_hio.c: In function 'mac_share_bind':
../../common/io/mac/mac_hio.c:121:22: error: comparison between pointer and integer [-Werror]
  if (mcip->mci_share == NULL) {
                      ^~
../../common/io/mac/mac_hio.c: In function 'mac_share_unbind':
../../common/io/mac/mac_hio.c:171:22: error: comparison between pointer and integer [-Werror]
  if (mcip->mci_share == NULL) {
                      ^~

../../common/io/mac/mac_datapath_setup.c: In function 'mac_next_bind_cpu':
../../common/io/mac/mac_datapath_setup.c:553:9: error: return makes integer from pointer without a cast [-Werror=int-conversion]
  return (NULL);
         ^
../../common/io/mac/mac_datapath_setup.c: In function 'mac_datapath_setup':
../../common/io/mac/mac_datapath_setup.c:2936:33: error: comparison between pointer and integer [-Werror]
        (rxhw || mcip->mci_share != NULL)) {
                                 ^~
../../common/io/mac/mac_datapath_setup.c:2962:33: error: comparison between pointer and integer [-Werror]
        (txhw || mcip->mci_share != NULL)) {
                                 ^~

../../common/io/mac/mac_client.c: In function 'mac_client_open':
../../common/io/mac/mac_client.c:1450:18: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
  mcip->mci_share = NULL;
                  ^
../../common/io/mac/mac_client.c: In function 'mac_client_set_rings_prop':
../../common/io/mac/mac_client.c:1705:22: error: comparison between pointer and integer [-Werror]
  if (mcip->mci_share != NULL)
                      ^~
In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/conf.h:37,
                 from ../../common/io/mac/mac_client.c:109:
../../common/io/mac/mac_client.c: In function 'mac_tx':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_client.c:3437:28: note: in expansion of macro 'NULL'
  mac_tx_cookie_t  cookie = NULL;
                            ^~~~
../../common/io/mac/mac_client.c:3454:11: error: return makes integer from pointer without a cast [-Werror=int-conversion]
    return (NULL);
           ^
../../common/io/mac/mac_client.c:3535:11: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
    cookie = NULL;
           ^
../../common/io/mac/mac_client.c: In function 'mac_tx_is_flow_blocked':
../../common/io/mac/mac_client.c:3605:14: error: comparison between pointer and integer [-Werror]
   if (cookie != NULL) {
              ^~

In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/callb.h:29,
                 from ../../common/io/mac/mac_sched.c:971:
../../common/io/mac/mac_sched.c: In function 'mac_tx_srs_no_desc':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3448:27: note: in expansion of macro 'NULL'
  mac_tx_cookie_t cookie = NULL;
                           ^~~~
../../common/io/mac/mac_sched.c: In function 'mac_tx_srs_enqueue':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3504:27: note: in expansion of macro 'NULL'
  mac_tx_cookie_t cookie = NULL;
                           ^~~~
../../common/io/mac/mac_sched.c: In function 'mac_tx_single_ring_mode':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3646:28: note: in expansion of macro 'NULL'
  mac_tx_cookie_t  cookie = NULL;
                            ^~~~
../../common/io/mac/mac_sched.c:3695:9: error: return makes integer from pointer without a cast [-Werror=int-conversion]
  return (NULL);
         ^
In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/callb.h:29,
                 from ../../common/io/mac/mac_sched.c:971:
../../common/io/mac/mac_sched.c: In function 'mac_tx_serializer_mode':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3712:28: note: in expansion of macro 'NULL'
  mac_tx_cookie_t  cookie = NULL;
                            ^~~~
../../common/sys/null.h:32:14: error: passing argument 4 of 'mac_tx_srs_enqueue' makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3728:13: note: in expansion of macro 'NULL'
       flag, NULL, ret_mp);
             ^~~~
../../common/io/mac/mac_sched.c:3501:1: note: expected 'uintptr_t {aka long unsigned int}' but argument is of type 'void *'
 mac_tx_srs_enqueue(mac_soft_ring_set_t *mac_srs, mblk_t *mp_chain,
 ^~~~~~~~~~~~~~~~~~
In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/callb.h:29,
                 from ../../common/io/mac/mac_sched.c:971:
../../common/sys/null.h:32:14: error: passing argument 4 of 'mac_tx_srs_enqueue' makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3747:23: note: in expansion of macro 'NULL'
       mp_chain, flag, NULL, ret_mp);
                       ^~~~
../../common/io/mac/mac_sched.c:3501:1: note: expected 'uintptr_t {aka long unsigned int}' but argument is of type 'void *'
 mac_tx_srs_enqueue(mac_soft_ring_set_t *mac_srs, mblk_t *mp_chain,
 ^~~~~~~~~~~~~~~~~~
../../common/io/mac/mac_sched.c:3759:13: error: comparison between pointer and integer [-Werror]
  if (cookie == NULL)
             ^~
In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/callb.h:29,
                 from ../../common/io/mac/mac_sched.c:971:
../../common/io/mac/mac_sched.c: In function 'mac_tx_fanout_mode':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3791:28: note: in expansion of macro 'NULL'
  mac_tx_cookie_t  cookie = NULL;
                            ^~~~
../../common/io/mac/mac_sched.c:3856:10: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
   cookie = NULL;
          ^
In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/callb.h:29,
                 from ../../common/io/mac/mac_sched.c:971:
../../common/io/mac/mac_sched.c: In function 'mac_tx_bw_mode':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:3876:28: note: in expansion of macro 'NULL'
  mac_tx_cookie_t  cookie = NULL;
                            ^~~~
../../common/io/mac/mac_sched.c:3966:10: error: return makes integer from pointer without a cast [-Werror=int-conversion]
   return (NULL);
          ^
../../common/io/mac/mac_sched.c: In function 'mac_tx_aggr_mode':
../../common/io/mac/mac_sched.c:4004:10: error: return makes integer from pointer without a cast [-Werror=int-conversion]
   return (NULL);
          ^
In file included from ../../common/sys/param.h:48:0,
                 from ../../common/sys/t_lock.h:38,
                 from ../../common/sys/callb.h:29,
                 from ../../common/io/mac/mac_sched.c:971:
../../common/io/mac/mac_sched.c: In function 'mac_tx_sring_enqueue':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:4818:27: note: in expansion of macro 'NULL'
  mac_tx_cookie_t cookie = NULL;
                           ^~~~
../../common/io/mac/mac_sched.c: In function 'mac_tx_soft_ring_process':
../../common/sys/null.h:32:14: error: initialization makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../../common/io/mac/mac_sched.c:4902:27: note: in expansion of macro 'NULL'
  mac_tx_cookie_t cookie = NULL;
                           ^~~~
../../common/io/mac/mac_sched.c:4992:10: error: return makes integer from pointer without a cast [-Werror=int-conversion]
   return (NULL);
          ^

../../common/io/mac/mac.c: In function 'i_mac_group_allocate_rings':
../../common/io/mac/mac.c:6166:12: error: comparison between pointer and integer [-Werror]
  if (share != NULL) {
            ^~
../../common/io/mac/mac.c:6275:12: error: comparison between pointer and integer [-Werror]
  if (share != NULL)
            ^~
../../common/io/mac/mac.c: In function 'mac_reserve_rx_group':
../../common/io/mac/mac.c:6464:28: error: comparison between pointer and integer [-Werror]
     if (gclient->mci_share == NULL &&
                            ^~
../../common/io/mac/mac.c:6511:15: error: comparison between pointer and integer [-Werror]
     if (share != NULL) {
               ^~
../../common/io/mac/mac.c:6598:15: error: comparison between pointer and integer [-Werror]
     if (share != NULL) {
               ^~
../../common/io/mac/mac.c: In function 'mac_release_rx_group':
../../common/io/mac/mac.c:6661:22: error: comparison between pointer and integer [-Werror]
  if (mcip->mci_share != NULL) {
                      ^~
../../common/io/mac/mac.c: In function 'mac_reserve_tx_group':
../../common/io/mac/mac.c:7002:28: error: comparison between pointer and integer [-Werror]
     if (gclient->mci_share == NULL &&
                            ^~
../../common/io/mac/mac.c:7070:15: error: comparison between pointer and integer [-Werror]
     if (share != NULL) {
               ^~
../../common/io/mac/mac.c:7104:12: error: comparison between pointer and integer [-Werror]
  if (share != NULL) {
            ^~
../../common/io/mac/mac.c: In function 'mac_release_tx_group':
../../common/io/mac/mac.c:7138:12: error: comparison between pointer and integer [-Werror]
  if (share != NULL)
            ^~
cc1: all warnings being treated as errors


  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
andy_js
  1. Ship It!
  2. 
      
wiedi
  1. Ship It!
  2. 
      
pmooney
  1. Looks good, save for the one question about mac_next_bind_cpu.

  2. This seems to be treated like an error condition, although it amounts to a default to cpu_id 0. It probably warrants further investigation, rather than smoothing over a compiler warning which does indicate a potential issue.

    1. um, well... they do use -1 to denote unusable cpuid, assuming we should return error, -1 should be used there. I am not sure at all what does happen if we actually do return -1 there...

    2. I spent the last two hours going over every consumer of mac_next_bind_cpu() and traced how its return value is used. My takeaway is that we should return -1, not 0. The consumers of this value (namely MAC's Tx/Rx CPU binding of soft rings/workers/polling/interrupts) all treat -1 as "no bind" or "no CPU found". It would treat the value 0 as "bind to CPU 0". This is not desirable for three reaaons:

      1. CPU 0 may be offline.

      2. CPU 0 may not be a member of the CPU partition assigned to this flow.

      3. We could end up piling many many threads and interrupts onto CPU 0.

      All of the consumers of this value end up calling cpu_get() first. This value is always checked against NULL and if so the binding operation is not performed. Furtermore, the only time mac_next_bind_cpu() would return -1 is if a CPU partition was defined but all of its CPUs are offline -- if that's the case then we have much bigger things to worry about I'm sure. So really I don't think this value is ever actually returned, but if we have to put a value there I strongly believe it should be -1 to indicate "no CPU found" and let the rest of MAC decide what it wants to do (which in this case means just don't bind the thread/interrupt).

  3. 
      
tsoome
andy_js
  1. Ship It!
  2. 
      
wiedi
  1. Ship It!
  2. 
      
pmooney
  1. 
      
  2. usr/src/uts/common/io/mac/mac_datapath_setup.c (Diff revisions 1 - 2)
     
     

    Something like "No matching CPU found online" might be more accurate, since in some cases is the CPU partition constraint that would cause the search to fail.

  3. 
      
tsoome
wiedi
  1. Ship It!
  2. 
      
pmooney
  1. Ship It!
  2. 
      
rzezeski
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...