On 1/22/08, Jean Delvare <[EMAIL PROTECTED]> wrote:
> Hi Jon,
>
> On Mon, 21 Jan 2008 19:11:23 -0500, Jon Smirl wrote:
> > i2c_new_device() is not propagating error codes up. Callers will need
> > to be fixed to use PTR_ERR() to recover the errors.
> >
> > struct i2c_client *
> > i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
> > {
> >       struct i2c_client       *client;
> >       int                     status;
> >
> >       client = kzalloc(sizeof *client, GFP_KERNEL);
> >       if (!client)
> > --should return -ENOMEM;
> >               return NULL;
> >
> >       client->adapter = adap;
> >
> >       client->dev.platform_data = info->platform_data;
> >       device_init_wakeup(&client->dev, info->flags & I2C_CLIENT_WAKE);
> >
> >       client->flags = info->flags & ~I2C_CLIENT_WAKE;
> >       client->addr = info->addr;
> >       client->irq = info->irq;
> >
> >       strlcpy(client->name, info->type, sizeof(client->name));
> >
> >       /* a new style driver may be bound to this device when we
> >        * return from this function, or any later moment (e.g. maybe
> >        * hotplugging will load the driver module).  and the device
> >        * refcount model is the standard driver model one.
> >        */
> >       status = i2c_attach_client(client);
> > --error status is not propagated up
> >       if (status < 0) {
> >               kfree(client);
> >               client = NULL;
> >       }
> >       return client;
> > }
> > EXPORT_SYMBOL_GPL(i2c_new_device);
>
> That's an implementation decision. The PTR_ERR() mechanism is not
> mandatory to use, I guess we didn't see a benefit in using it for this
> function. I can't think of a recoverable error this function could
> return, and if all errors are unrecoverable there's little point in
> being able to distinguish between then, is there? If you have a use
> case where the actual error code returned by i2c_new_device() would
> lead the caller to do something different, please present it to us and
> we'll discuss it.

Passing up the actual error code is helpful to humans in locating the
cause of the error. If I have the exact error I can search lower in
the source code for where it was generated. In my code that calls this
function I was going to print out the error result, but it is
pointless if the error is always NULL. Right now if this function
fails I have no clue as to why it failed, I will have to go document
it with printfs and try to reproduce.

>
> --
> Jean Delvare
>


-- 
Jon Smirl
[EMAIL PROTECTED]

_______________________________________________
i2c mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/i2c

Reply via email to