* [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check
@ 2005-03-24 3:14 Adrian Bunk
2005-03-24 5:13 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Bunk @ 2005-03-24 3:14 UTC (permalink / raw)
To: vojtech; +Cc: linux-kernel, linux-input
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 <bunk@stusta.de>
--- linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c.old 2005-03-24 02:37:08.000000000 +0100
+++ linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100
@@ -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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check
2005-03-24 3:14 [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check Adrian Bunk
@ 2005-03-24 5:13 ` Dmitry Torokhov
2005-03-24 21:26 ` Adrian Bunk
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2005-03-24 5:13 UTC (permalink / raw)
To: linux-input; +Cc: Adrian Bunk, vojtech, linux-kernel
On Wednesday 23 March 2005 22:14, Adrian Bunk wrote:
> 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 <bunk@stusta.de>
>
> --- linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c.old 2005-03-24 02:37:08.000000000 +0100
> +++ linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100
> @@ -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;
>
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.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check
2005-03-24 5:13 ` Dmitry Torokhov
@ 2005-03-24 21:26 ` Adrian Bunk
2005-03-24 21:39 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Adrian Bunk @ 2005-03-24 21:26 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, vojtech, linux-kernel
On Thu, Mar 24, 2005 at 12:13:16AM -0500, Dmitry Torokhov wrote:
> On Wednesday 23 March 2005 22:14, Adrian Bunk wrote:
> > 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 <bunk@stusta.de>
> >
> > --- linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c.old 2005-03-24 02:37:08.000000000 +0100
> > +++ linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100
> > @@ -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;
> >
>
> 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?
> Dmitry
cu
Adrian
--
"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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check
2005-03-24 21:26 ` Adrian Bunk
@ 2005-03-24 21:39 ` Dmitry Torokhov
2005-03-25 0:11 ` Adrian Bunk
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2005-03-24 21:39 UTC (permalink / raw)
To: Adrian Bunk; +Cc: linux-input, vojtech, linux-kernel
On Thu, 24 Mar 2005 22:26:02 +0100, Adrian Bunk <bunk@stusta.de> wrote:
> On Thu, Mar 24, 2005 at 12:13:16AM -0500, Dmitry Torokhov wrote:
> > On Wednesday 23 March 2005 22:14, Adrian Bunk wrote:
> > > 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 <bunk@stusta.de>
> > >
> > > --- linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c.old 2005-03-24 02:37:08.000000000 +0100
> > > +++ linux-2.6.12-rc1-mm1-full/drivers/input/serio/libps2.c 2005-03-24 02:38:28.000000000 +0100
> > > @@ -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;
> > >
> >
> > 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?
>
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.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check
2005-03-24 21:39 ` Dmitry Torokhov
@ 2005-03-25 0:11 ` Adrian Bunk
0 siblings, 0 replies; 5+ messages in thread
From: Adrian Bunk @ 2005-03-25 0:11 UTC (permalink / raw)
To: dtor_core; +Cc: linux-input, vojtech, linux-kernel
On Thu, Mar 24, 2005 at 04:39:31PM -0500, Dmitry Torokhov wrote:
>
> 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.
> Dmitry
cu
Adrian
--
"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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2005-03-25 0:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-24 3:14 [2.6 patch] drivers/input/serio/libps2.c: ps2_command: add a missing check Adrian Bunk
2005-03-24 5:13 ` Dmitry Torokhov
2005-03-24 21:26 ` Adrian Bunk
2005-03-24 21:39 ` Dmitry Torokhov
2005-03-25 0:11 ` Adrian Bunk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox