11319 devfsadm: NULL pointer errors

Review Request #2056 — Created July 4, 2019 and submitted July 7, 2019, 8:10 p.m.

Information
Owner:	tsoome
Repository:	illumos-gate
Branch:
Bugs:	11319
Depends On:
Commit:	18013e8...
Reviewers
Groups:	general
People:

Description

../cfg_link.c: In function 'pci_cfg_ap_path':
../cfg_link.c:730:26: error: comparison between pointer and integer [-Werror]
    if ((curnode == node) != NULL)
                          ^~

In file included from /code/illumos-gate/proto/root_i386/usr/include/iso/string_iso.h:50:0,
                 from /code/illumos-gate/proto/root_i386/usr/include/string.h:33,
                 from ../devfsadm.c:38:
../devfsadm.c: In function 'main':
/code/illumos-gate/proto/root_i386/usr/include/sys/null.h:28:14: error: passing argument 2 of 'thr_create' makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../devfsadm.c:380:25: note: in expansion of macro 'NULL'
    if (thr_create(NULL, NULL,
                         ^~~~
In file included from /code/illumos-gate/proto/root_i386/usr/include/libsysevent.h:29:0,
                 from ../devfsadm_impl.h:67,
                 from ../devfsadm.c:47:
/code/illumos-gate/proto/root_i386/usr/include/thread.h:54:12: note: expected 'size_t {aka unsigned int}' but argument is of type 'void *'
 extern int thr_create(void *, size_t, void *(*)(void *), void *, long,
            ^~~~~~~~~~
In file included from /code/illumos-gate/proto/root_i386/usr/include/iso/string_iso.h:50:0,
                 from /code/illumos-gate/proto/root_i386/usr/include/string.h:33,
                 from ../devfsadm.c:38:
/code/illumos-gate/proto/root_i386/usr/include/sys/null.h:28:14: error: passing argument 2 of 'thr_create' makes integer from pointer without a cast [-Werror=int-conversion]
 #define NULL ((void *)0)
              ^
../devfsadm.c:392:25: note: in expansion of macro 'NULL'
    if (thr_create(NULL, NULL,
                         ^~~~
In file included from /code/illumos-gate/proto/root_i386/usr/include/libsysevent.h:29:0,
                 from ../devfsadm_impl.h:67,
                 from ../devfsadm.c:47:
/code/illumos-gate/proto/root_i386/usr/include/thread.h:54:12: note: expected 'size_t {aka unsigned int}' but argument is of type 'void *'
 extern int thr_create(void *, size_t, void *(*)(void *), void *, long,
            ^~~~~~~~~~
../devfsadm.c: In function 'parse_args':
../devfsadm.c:707:43: error: comparison between pointer and integer [-Werror]
    if ((mc.major == -1) || (mc.drvname[0] == NULL)) {
                                           ^~
../devfsadm.c: In function 'create_selector_list':
../devfsadm.c:6346:20: error: comparison between pointer and integer [-Werror]
  while ((*selector != NULL) && (error == FALSE)) {
                    ^~
../devfsadm.c: In function 'getnexttoken':
../devfsadm.c:7549:8: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
    *cp = NULL; /* terminate token */
        ^
cc1: all warnings being treated as errors

Testing Done

Issues

0
0
0
1
All Issues: 1

Description	From	Last Updated
I have grave doubts about this. I suspect that the original author meant it to read: if ((curnode = node)) ...	gdamore	July 6, 2019, 9:42 a.m.

usr/src/cmd/devfsadm/cfg_link.c (Diff revision 1)

Show all issues

I have grave doubts about this.
I suspect that the original author meant it to read:
if ((curnode = node)) != NULL)
(meaning he used comparision instead of assignment.)
If he meant what you wrote here, it would have been clearer to write this as:
if (curnode != node)
This will need some investigation before it can be approved, I think.  I also think this is one of the cases where the expanded warnings may have found a real bug.

tsoome July 5, 2019, 6:23 a.m.

I am still not 100% sure, but it does not seem to be an assignment. We came in with argument node (assigned to curnode), we did got a chassis node, and only if this is a first pass of this loop (curnode == node), we do set chasflag. I think, we really just need to remove '!= 0' expression there.