Re: [PATCH v2 00/32] driver core: Constify API device_find_child() and adapt for various existing usages

[James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>]

On Wed, 2024-12-04 at 20:26 +0800, Zijun Hu wrote:
> On 2024/12/3 23:34, James Bottomley wrote:
> > > > This also enables an incremental migration.
> > > change the API prototype from:
> > > device_find_child(..., void *data_0, int (*match)(struct device
> > > *dev, void *data));
> > > 
> > > to:
> > > device_find_child(..., const void *data_0, int (*match)(struct
> > > device *dev, const void *data));
> > > 
> > > For @data_0,  void * -> const void * is okay.
> > > but for @match, the problem is function pointer type
> > > incompatibility.
> > > 
> > > there are two solutions base on discussions.
> > > 
> > > 1) squashing likewise Greg mentioned.
> > >    Do all of the "prep work" first, and then
> > >    do the const change at the very end, all at once.
> > > 
> > > 2)  as changing platform_driver's remove() prototype.
> > > Commit: e70140ba0d2b ("Get rid of 'remove_new' relic from
> > > platform driver struct")
> > > 
> > >  introduce extra device_find_child_new() which is constified  ->
> > > use *_new() replace ALL device_find_child() instances one by one
> > > -> remove device_find_child() -> rename *_new() to
> > > device_find_child() once.
> > Why bother with the last step, which churns the entire code base
> > again?
> 
> keep the good API name device_find_child().

Well, I think it's a good opportunity to rename the API better, but if
that's the goal, you can still do it with _Generic() without churning
the code base a second time.  The example is in
slab.h:kmem_cache_create

> > Why not call the new function device_find_child_const() and simply
> > keep it (it's descriptive of its function).  That way you can have
> > a patch series without merging and at the end simply remove the old
> > function.
> 
> device_find_child is a good name for the API, 'find' already means
> const.

Not to me it doesn't, but that's actually not what I think is wrong
with the API name: it actually only returns the first match, so I'd
marginally prefer it to be called device_find_first_child() ... not
enough to churn the code to change it, but since you're doing that
anyway it might make sense as an update.

Regards,

James