Re: [PATCH v7 2/3] i2c: core: add device-managed version of i2c_new_dummy

[Wolfram Sang <wsa@the-dreams.de>]

> I personally really dislike the proposed name. It is akin to the abomination
> where some sort of abbreviation of the types of variables are included also
> in the variable names. It's useless clutter, at least to me.

In general, I agree with you...

> I can see that you do not want to change the i2c_new_{device,dummy} names
> since they are kind of widespread and I can also see that you want to
> match the devm_ name with the non-devm_ name. So, I see *why* this naming
> is as it is, but I just don't like it.

... yet, there is another reason which is consistency. i2c_new_{device,dummy}
return NULL if something went wrong, and if the devm_* variants return
an ERRPTR, this is super confusing and error prone IMO. And the
difference between 'i2c_new_dummy' and 'i2c_register_dummy' doesn't make
that more clear, I think.

I would love to convert all of those functions to return an errptr at
some time. However, they are so widespread that this is difficult. I
actually had a look to convert the users with coccinelle, but handling
the error cases is so diverse that I don't think this is a way forward.

> I had a look at a couple of the i2c_new_dummy call sites, and some of them
> can be easily updated to simply pass on the PTRERR instead of hardcoding
> INVAL (or something) on NULL, some of them ignore the return value (and are
> thus not able to call i2c_unregister_device correctly) and some are

Those are likely prime candidates to convert to devm_*.

> different in other ways. In short, it seems that there are plenty of good
> opportunities to update lots of call sites to the new interfaces.

Yes. But it is quite some work, even more with i2c_new_device.

> However, after most of the maintained uses have been updated we are bound
> to have a tail of unmaintained code that will use the old interface. At
> some point, someone might convert the tail of call sites using the old
> NULL-returning functions. At that point we are stuck with a bunch of ugly
> hysterical foo_errptr names. And again it will be a daunting series to
> change them all at once.

OK, I can see that. If the longterm goal is to return errnos, then
it doesn't make sense to have it in the function name. This is valid
criticism. Yet, I still keep the other criticism from the first
paragraph where i2c_new_dummy and i2c_register_dummy is confusing.
Unless someone comes up with a solution we all overlooked, it will
probably be chosing the lesser evil.