On Tue, 2024-12-03 at 22:56 +0800, Zijun Hu wrote:

On 2024/12/3 22:07, Thomas Weißschuh wrote:

quoted

On 2024-12-03 08:58:26-0500, James Bottomley wrote:

quoted

On Tue, 2024-12-03 at 21:02 +0800, Zijun Hu wrote:

quoted

On 2024/12/3 20:41, Greg Kroah-Hartman wrote:

quoted

On Tue, Dec 03, 2024 at 08:23:45PM +0800, Zijun Hu wrote:

[...]

quoted

quoted

quoted

or squash such patch series into a single patch ?

various subsystem maintainers may not like squashing way.

Agreed, so look into either doing it in a bisectable way if
at all possible.  As I don't see a full series here, I can't
suggest how it needs to happen :(

let me send you a full series later and discuss how to solve
this issue.

It's only slightly more complex than what we normally do: modify
all instances and then change the API.  In this case you have an
additional problem because the prototype "const void *" will
cause a mismatch if a function has "void *".  The easiest way to
solve this is probably to make device_find_child a macro that
coerces its function argument to having a non const "void *" and
then passes off to the real function.  If you do that in the
first patch, then you can constify all the consumers and finally
remove the macro coercion in the last patch.

Casting function pointers like that should be detected and trapped
by control flow integrity checking (KCFI).

Another possibility would be to use a macro and _Generic to
dispatch to two different backing functions. See __BIN_ATTR() in
include/linux/sysfs.h for an inspiration.

That's way over complicated for this conversion: done properly there
should be no need for _Generic() compile time type matching at all.

this way may fix building error issue but does not achieve our
purpose. our purpose is that there are only constified
device_find_child().

quoted

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?
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.

Regards,

James