Skip to contentSkip to conversation

drivers/input/serio/libps2.c: ps2_command: add a missing check

PATCH In review Adrian Bunk 3 participants 5 messages Mar 24, 2005 – Mar 25, 2005 Input drivers

lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 drivers/input/serio/libps2.c: ps2_command: add a missing check

The Coverity checker noted that while all other uses of param in ps2_command() were guarded by a NULL check, this one wasn't.

Signed-off-by Adrian Bunk <[email protected]>
drivers/input/serio/libps2.c renamed from drivers/input/serio/libps2.c.old +4 -3
@@ -106,9 +106,10 @@ int ps2_command(struct ps2dev *ps2dev, u
 			command == PS2_CMD_RESET_BAT ? 1000 : 200))
 			goto out;
 
-	for (i = 0; i < send; i++)
-		if (ps2_sendbyte(ps2dev, param[i], 200))
-			goto out;
+	if (param)
+		for (i = 0; i < send; i++)
+			if (ps2_sendbyte(ps2dev, param[i], 200))
+				goto out;
 
 	/*
 	 * The reset command takes a long time to execute.
#1 lore raw
Quoted 9 lines · Adrian Bunk

The Coverity checker noted that while all other uses of param in ps2_command() were guarded by a NULL check, this one wasn't.

Signed-off-by Adrian Bunk <[email protected]>
drivers/input/serio/libps2.c renamed from drivers/input/serio/libps2.c.old +0 -0
@@ -106,9 +106,10 @@ int ps2_command(struct ps2dev *ps2dev, u
 			command == PS2_CMD_RESET_BAT ? 1000 : 200))

drivers/input/serio/libps2.c int ps2_command(struct ps2dev *ps2dev, u · line 113

Quoted 10 lines
drivers/input/serio/libps2.c renamed from drivers/input/serio/libps2.c.old +4 -3
 			goto out;
 
-	for (i = 0; i < send; i++)
-		if (ps2_sendbyte(ps2dev, param[i], 200))
-			goto out;
+	if (param)
+		for (i = 0; i < send; i++)
+			if (ps2_sendbyte(ps2dev, param[i], 200))
+				goto out;

I somewhat disagree on this one. If caller specified that command requires arguments to be sent and it does not provide them I'd rather had it OOPS on the spot. With receiving, however, caller does not really have control over number of characters coming from the device so specifying NULL allows just ignore whatever response there is.

Signature · 1 line
Dmitry
author
#2 lore raw
Quoted 12 lines · Dmitry Torokhov
Quoted 11 lines · Adrian Bunk

The Coverity checker noted that while all other uses of param in ps2_command() were guarded by a NULL check, this one wasn't.

Signed-off-by Adrian Bunk <[email protected]>
drivers/input/serio/libps2.c renamed from drivers/input/serio/libps2.c.old +0 -0
@@ -106,9 +106,10 @@ int ps2_command(struct ps2dev *ps2dev, u
 			command == PS2_CMD_RESET_BAT ? 1000 : 200))
 			goto out;
Quoted 14 lines

drivers/input/serio/libps2.c int ps2_command(struct ps2dev *ps2dev, u · line 113

Quoted 8 lines
drivers/input/serio/libps2.c renamed from drivers/input/serio/libps2.c.old +4 -3
-	for (i = 0; i < send; i++)
-		if (ps2_sendbyte(ps2dev, param[i], 200))
-			goto out;
+	if (param)
+		for (i = 0; i < send; i++)
+			if (ps2_sendbyte(ps2dev, param[i], 200))
+				goto out;

I somewhat disagree on this one. If caller specified that command requires arguments to be sent and it does not provide them I'd rather had it OOPS on the spot. With receiving, however, caller does not really have control over number of characters coming from the device so specifying NULL allows just ignore whatever response there is.

Understood.

Could this be handled with a BUG_ON?

Quoted 1 line

Dmitry

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
maintainer
#3 lore raw
Quoted 28 lines · Adrian Bunk
Quoted 12 lines · Dmitry Torokhov
Quoted 11 lines · Adrian Bunk

The Coverity checker noted that while all other uses of param in ps2_command() were guarded by a NULL check, this one wasn't.

Signed-off-by Adrian Bunk <[email protected]>
drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100 renamed from drivers/input/serio/libps2.c.old 2005-03-24 02:37:08.000000000 +0100 +0 -0
@@ -106,9 +106,10 @@ int ps2_command(struct ps2dev *ps2dev, u
                    command == PS2_CMD_RESET_BAT ? 1000 : 200))
                    goto out;
Quoted 14 lines

drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100 int ps2_command(struct ps2dev *ps2dev, u · line 113

Quoted 8 lines
drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100 renamed from drivers/input/serio/libps2.c.old 2005-03-24 02:37:08.000000000 +0100 +4 -3
-   for (i = 0; i < send; i++)
-           if (ps2_sendbyte(ps2dev, param[i], 200))
-                   goto out;
+   if (param)
+           for (i = 0; i < send; i++)
+                   if (ps2_sendbyte(ps2dev, param[i], 200))
+                           goto out;

I somewhat disagree on this one. If caller specified that command requires arguments to be sent and it does not provide them I'd rather had it OOPS on the spot. With receiving, however, caller does not really have control over number of characters coming from the device so specifying NULL allows just ignore whatever response there is.

Quoted 4 lines

Understood.

Could this be handled with a BUG_ON?

Yes, if it will make the checker happy. Although I (and this is probably just me) use BUG_ON only if kernel would not OOPS otherwise, to avoid scribbling over random memory and such.

Signature · 1 line
Dmitry
author
#4 lore raw
Quoted 4 lines · Dmitry Torokhov

Yes, if it will make the checker happy. Although I (and this is probably just me) use BUG_ON only if kernel would not OOPS otherwise, to avoid scribbling over random memory and such.

I don't whether it would make the checker happy (and I don't care much).

It would IMHO make the code better understandable.

But I don't have a very strong opinion on this issue.

Quoted 1 line

Dmitry

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed