Re: [PATCH 6/12] drivers/platform/x86: Correct redundant test

From: Julia Lawall
Date: Tue Jul 28 2009 - 16:12:25 EST


On Tue, 28 Jul 2009, Paulo Marques wrote:

> Julia Lawall wrote:
> > [...]
> > ---
> > drivers/platform/x86/fujitsu-laptop.c | 3 ---
> > 1 files changed, 0 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > index 218b9a1..5306901 100644
> > --- a/drivers/platform/x86/fujitsu-laptop.c
> > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > @@ -745,9 +745,6 @@ static int acpi_fujitsu_remove(struct acpi_device *device, int type)
> >
> > fujitsu = acpi_driver_data(device);
> >
> > - if (!device || !acpi_driver_data(device))
> > - return -EINVAL;
> > -
>
> Shouldn't this still do a:
>
> if (!fujitsu)
> return -EINVAL;

acpi_driver_data just accesses a field of its argument. Is there a worry
that from one call to the next it could have a different value?

Perhaps it would be better to first test !device, then initialize fujitsu,
and then test the result of fujitsu? The acpi_driver_data, which might
someday do something more complicated, would only be called once.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/