[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024082419-aspirate-pediatric-ee88@gregkh>
Date: Sat, 24 Aug 2024 11:23:29 +0800
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Davidlohr Bueso <dave@...olabs.net>,
Jonathan Cameron <jonathan.cameron@...wei.com>,
Dave Jiang <dave.jiang@...el.com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>,
Ira Weiny <ira.weiny@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Takashi Sakamoto <o-takashi@...amocchi.jp>,
Timur Tabi <timur@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org,
linux1394-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH v2 1/4] driver core: Make parameter check consistent for
API cluster device_(for_each|find)_child()
On Thu, Aug 15, 2024 at 10:58:02PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@...cinc.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@...cinc.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
Powered by blists - more mailing lists