Re: [PATCH 2/5] driver core: Introduce an API constify_device_find_child_helper()

From: Greg Kroah-Hartman
Date: Tue Aug 13 2024 - 06:58:44 EST


On Tue, Aug 13, 2024 at 06:50:04PM +0800, quic_zijuhu wrote:
> On 8/13/2024 5:45 PM, Greg Kroah-Hartman wrote:
> > On Sun, Aug 11, 2024 at 08:18:08AM +0800, Zijun Hu wrote:
> >> From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
> >>
> >> Introduce constify_device_find_child_helper() to replace existing
> >> device_find_child()'s usages whose match functions will modify
> >> caller's match data.
> >
> > Ick, that's not a good name, it should be "noun_verb" with the subsystem being on the prefix always.
> >
> okay, got it.
>
> is it okay to use device_find_child_mut() suggested by Przemek Kitszel ?

No, just switch all callers over to be const and keep the same name.

> > But why is this even needed? Device pointers are NOT const for the
> > obvious reason that they can be changed by loads of different things.
> > Trying to force them to be const is going to be hard, if not impossible.
> >
>
> [PATCH 3/5] have more discussion about these questions with below link:
> https://lore.kernel.org/all/8b8ce122-f16b-4207-b03b-f74b15756ae7@xxxxxxxxxx/
>
>
> The ultimate goal is to make device_find_child() have below prototype:
>
> struct device *device_find_child(struct device *dev, const void *data,
> int (*match)(struct device *dev, const void *data));
>
> Why ?
>
> (1) It does not make sense, also does not need to, for such device
> finding operation to modify caller's match data which is mainly
> used for comparison.
>
> (2) It will make the API's match function parameter have the same
> signature as all other APIs (bus|class|driver)_find_device().
>
>
> My idea is that:
> use device_find_child() for READ only accessing caller's match data.
>
> use below API if need to Modify caller's data as
> constify_device_find_child_helper() does.
> int device_for_each_child(struct device *dev, void *data,
> int (*fn)(struct device *dev, void *data));
>
> So the The ultimate goal is to protect caller's *match data* @*data NOT
> device @*dev.

Ok, sorry, I was confused.

thanks,

greg k-h