linux-kernel - Re: [PATCH v2 05/13] i2c: acpi: Return error pointers from i2c_acpi_new

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Nov 2018 11:04:32 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Darren Hart <dvhart@...radead.org>,
        platform-driver-x86@...r.kernel.org,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        linux-acpi@...r.kernel.org, Jonathan Cameron <jic23@...nel.org>,
        Wolfram Sang <wsa@...-dreams.de>, linux-i2c@...r.kernel.org,
        Hans de Goede <hdegoede@...hat.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 05/13] i2c: acpi: Return error pointers from
 i2c_acpi_new_device()

On Mon, Nov 26, 2018 at 05:08:50PM +0200, Andy Shevchenko wrote:
> The caller would like to know the reason why the i2c_acpi_new_device() fails.
> For example, if adapter is not available, it might be in the future and we
> would like to re-probe the clients again. But at the same time we would like to
> bail out if the error seems unrecoverable, such as out of memory condition.
> To achieve this, return error pointer in some cases.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
> ---
>  drivers/i2c/i2c-core-acpi.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index 32affd3fa8bd..af4b5bd5d973 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -387,6 +387,7 @@ struct notifier_block i2c_acpi_notifier = {
>   * Also see i2c_new_device, which this function calls to create the i2c-client.
>   *
>   * Returns a pointer to the new i2c-client, or NULL if the adapter is not found.
> + * In some cases might return an error pointer.

I would rather make it return error pointer always. Then the caller can
just check for IS_ERR() and not need to deal with the possible NULL. It
is also more consistent that way than saying "some cases might return an
error pointer" (but some cases you get NULL or even pointer to the
created object) ;-)