On Thursday 10 September 2009 10:43:25 am Alan Jenkins wrote:
> I noticed a pattern of unnecessary checks, and wrote some semantic
> patches to remove them (using the spatch tool). Here are the
> results, modulo some manual adjustment of blank lines to try and
> preserve the coding style in different files.
>
> Would you be interested in accepting this? Can I submit it like
> this, or should I break it up somehow?
I'm generally in favor of removing these checks. In most cases,
you can go even farther, for example:
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index 98b9690..57fbf70 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -259,9 +259,6 @@ static int acpi_ac_add(struct acpi_device *device)
> struct acpi_ac *ac = NULL;
The initialization of "ac" is useless since the first thing we
do below is assign to it.
> - if (!device)
> - return -EINVAL;
> -
> ac = kzalloc(sizeof(struct acpi_ac), GFP_KERNEL);
> if (!ac)
> return -ENOMEM;
> @@ -306,11 +303,10 @@ static int acpi_ac_add(struct acpi_device *device)
>
> static int acpi_ac_resume(struct acpi_device *device)
> {
> - struct acpi_ac *ac;
> + struct acpi_ac *ac = acpi_driver_data(device);
> unsigned old_state;
> - if (!device || !acpi_driver_data(device))
> + if (!ac)
> return -EINVAL;
I would also remove the test of "!ac". acpi_ac_resume() can only
be called after acpi_ac_add() completes successfully, and acpi_ac_add()
always sets "device->driver_data = ac".
So testing "!ac" here will only find memory corruption or a bug in
the Linux/ACPI core. In either case, I think it's better to oops
than to return -EINVAL, which will probably get ignored.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
