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]>
--- 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.
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 <[email protected]>
>
> --- 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
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 <[email protected]>
> >
> > --- 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
On Thu, 24 Mar 2005 22:26:02 +0100, Adrian Bunk <[email protected]> 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 <[email protected]>
> > >
> > > --- 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
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