Re: [PATCH v2 1/4] driver core: Make parameter check consistent for API cluster device_(for_each|find)_child()

Greg Kroah-Hartman
2024-08-24T05:18:35Z

On Thu, Aug 15, 2024 at 10:58:02PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> The following API cluster takes the same type parameter list, but do not
> have consistent parameter check as shown below.
>
> device_for_each_child(struct device *parent, ...) // check (!parent->p)
> device_for_each_child_reverse(struct device *parent, ...) // same as above
> device_find_child(struct device *parent, ...) // check (!parent)
>
> Fixed by using consistent check (!parent || !parent->p) for the cluster.
>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
> drivers/base/core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 1688e76cb64b..b1dd8c5590dc 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4004,7 +4004,7 @@ int device_for_each_child(struct device *parent, void *data,
> struct device *child;
> int error = 0;
>
> - if (!parent->p)
> + if (!parent || !parent->p)
> return 0;
>
> klist_iter_init(&parent->p->klist_children, &i);
> @@ -4034,7 +4034,7 @@ int device_for_each_child_reverse(struct device *parent, void *data,
> struct device *child;
> int error = 0;
>
> - if (!parent->p)
> + if (!parent || !parent->p)
> return 0;
>
> klist_iter_init(&parent->p->klist_children, &i);
> @@ -4068,7 +4068,7 @@ struct device *device_find_child(struct device *parent, void *data,
> struct klist_iter i;
> struct device *child;
>
> - if (!parent)
> + if (!parent || !parent->p)
> return NULL;

My review comments that I just made previously were more "generic",
sorry. This change looks fine, there's no additional runtime overhead
for doing this check as we already have to dereference the pointer, so
might as well be consistant. I'll go queue this up next week when I get
back from conference travel.

thanks,

greg k-h