Skip to contentSkip to conversation

isdn: fix integer as NULL pointer warning

PATCH In review Harvey Harrison 2 participants 2 messages May 22, 2008 – May 23, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 3/5 isdn: fix integer as NULL pointer warning

drivers/isdn/hysdn/hycapi.c:465:42: warning: Using plain integer as NULL pointer drivers/isdn/hysdn/hycapi.c:467:44: warning: Using plain integer as NULL pointer drivers/isdn/hysdn/hycapi.c:469:42: warning: Using plain integer as NULL pointer

---
 drivers/isdn/hysdn/hycapi.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/isdn/hysdn/hycapi.c +3 -3
index d3999a8..53f6ad1 100644
@@ -462,11 +462,11 @@ static int hycapi_read_proc(char *page, char **start, off_t off,
 		default: s = "???"; break;
 	}
 	len += sprintf(page+len, "%-16s %s\n", "type", s);
-	if ((s = cinfo->version[VER_DRIVER]) != 0)
+	if ((s = cinfo->version[VER_DRIVER]) != NULL)
 		len += sprintf(page+len, "%-16s %s\n", "ver_driver", s);
-	if ((s = cinfo->version[VER_CARDTYPE]) != 0)
+	if ((s = cinfo->version[VER_CARDTYPE]) != NULL)
 		len += sprintf(page+len, "%-16s %s\n", "ver_cardtype", s);
-	if ((s = cinfo->version[VER_SERIAL]) != 0)
+	if ((s = cinfo->version[VER_SERIAL]) != NULL)
 		len += sprintf(page+len, "%-16s %s\n", "ver_serial", s);
     
 	len += sprintf(page+len, "%-16s %s\n", "cardname", cinfo->cardname);
Signature · 1 line
1.5.5.1.579.g4e43
#1 lore raw

drivers/isdn/hysdn/hycapi.c static int hycapi_read_proc(char *page, char **start, off_t off, · line 466

Quoted 4 lines · Harvey Harrison
drivers/isdn/hysdn/hycapi.c +1 -1
 	len += sprintf(page+len, "%-16s %s\n", "type", s);
-	if ((s = cinfo->version[VER_DRIVER]) != 0)
+	if ((s = cinfo->version[VER_DRIVER]) != NULL)
 		len += sprintf(page+len, "%-16s %s\n", "ver_driver", s);

For thigns like this (ie testing an assignment), I personally much prefer

	s = cinfo->version[VER_DRIVER];
	if (s)
		len += sprintf(page+len, "%-16s %s\n", "ver_driver", s);

over the uglier and unreadable version.

IOW, testing assignments is good only when:

- you have to do it because of syntax (ie notably in a "while()" loop)

- there's some reason you want it to be a single statement (eg doing a macro or other thing)

- of the assignment is really simple, and the test is not against NULL or zero.

The reason for that "the test is not against NULL or zero" is that testing for NULL and 0 is better done with just a "if (x)", and in an assignment that just means either (a) a incomprehensible extra parenthesis just to shut the compiler up or (b) changing the simple test into a stupid test (ie doing "if (x != NULL)").

(b) is much preferable to (a), but just doing it as two statements is much preferable to either!

Linus