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

[Greg Kroah-Hartman]

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