[RFC PATCH] usb: gadget: Drop unnecessary NULL checks after container_of

[RFC PATCH] usb: gadget: Drop unnecessary NULL checks after container_of

[Guenter Roeck]

The parameters passed to allow_link and drop_link functions are never NULL.
That means the result of container_of() on those parameters is also
never NULL, even if the reference into the structure points to the first
element of the structure. Remove the subsequent NULL checks.

The changes in this patch were made automatically using the following
Coccinelle script.

@@
type t;
identifier v;
statement s;
@@

<+...
(
  t v = container_of(...);
|
  v = container_of(...);
)
  ...
  when != v
- if (\( !v \| v == NULL \) ) s
...+>

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
After the recent discussion about a patch which tried to add a check
against NULL after container_of(), I realized that there are a number
of such checks in the kernel.

Now the big question: Are patches like this acceptable, or do they count
as noise ?

Guenter

 drivers/usb/gadget/function/uvc_configfs.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 00fb58e50a15..b9d1bcb4f4ff 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -914,8 +914,6 @@ static int uvcg_streaming_header_allow_link(struct config_item *src,
 
 	target_fmt = container_of(to_config_group(target), struct uvcg_format,
 				  group);
-	if (!target_fmt)
-		goto out;
 
 	uvcg_format_set_indices(to_config_group(target));
 
@@ -955,8 +953,6 @@ static void uvcg_streaming_header_drop_link(struct config_item *src,
 	mutex_lock(&opts->lock);
 	target_fmt = container_of(to_config_group(target), struct uvcg_format,
 				  group);
-	if (!target_fmt)
-		goto out;
 
 	list_for_each_entry_safe(format_ptr, tmp, &src_hdr->formats, entry)
 		if (format_ptr->fmt == target_fmt) {
-- 
2.17.1

Re: [RFC PATCH] usb: gadget: Drop unnecessary NULL checks after container_of

[Greg Kroah-Hartman]

On Fri, Apr 23, 2021 at 08:06:26AM -0700, Guenter Roeck wrote:
> The parameters passed to allow_link and drop_link functions are never NULL.
> That means the result of container_of() on those parameters is also
> never NULL, even if the reference into the structure points to the first
> element of the structure. Remove the subsequent NULL checks.
> 
> The changes in this patch were made automatically using the following
> Coccinelle script.
> 
> @@
> type t;
> identifier v;
> statement s;
> @@
> 
> <+...
> (
>   t v = container_of(...);
> |
>   v = container_of(...);
> )
>   ...
>   when != v
> - if (\( !v \| v == NULL \) ) s
> ...+>
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> After the recent discussion about a patch which tried to add a check
> against NULL after container_of(), I realized that there are a number
> of such checks in the kernel.
> 
> Now the big question: Are patches like this acceptable, or do they count
> as noise ?

Yes they are acceptable, and no, they are not noise.

I will be glad to take this after -rc1 is out, thanks.

thanks,

greg k-h

Re: [RFC PATCH] usb: gadget: Drop unnecessary NULL checks after container_of

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]


Hi Guenter,

Guenter Roeck <linux@roeck-us.net> writes:
> The parameters passed to allow_link and drop_link functions are never NULL.
> That means the result of container_of() on those parameters is also
> never NULL, even if the reference into the structure points to the first
> element of the structure. Remove the subsequent NULL checks.
>
> The changes in this patch were made automatically using the following
> Coccinelle script.
>
> @@
> type t;
> identifier v;
> statement s;
> @@
>
> <+...
> (
>   t v = container_of(...);
> |
>   v = container_of(...);
> )
>   ...
>   when != v
> - if (\( !v \| v == NULL \) ) s
> ...+>
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> After the recent discussion about a patch which tried to add a check
> against NULL after container_of(), I realized that there are a number
> of such checks in the kernel.
>
> Now the big question: Are patches like this acceptable, or do they count
> as noise ?

Not noise in my book :-)

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

Re: [RFC PATCH] usb: gadget: Drop unnecessary NULL checks after container_of

[Laurent Pinchart]

Hi Guenter,

On Sat, Apr 24, 2021 at 11:03:19AM +0300, Felipe Balbi wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> > The parameters passed to allow_link and drop_link functions are never NULL.
> > That means the result of container_of() on those parameters is also
> > never NULL, even if the reference into the structure points to the first
> > element of the structure. Remove the subsequent NULL checks.
> >
> > The changes in this patch were made automatically using the following
> > Coccinelle script.
> >
> > @@
> > type t;
> > identifier v;
> > statement s;
> > @@
> >
> > <+...
> > (
> >   t v = container_of(...);
> > |
> >   v = container_of(...);
> > )
> >   ...
> >   when != v
> > - if (\( !v \| v == NULL \) ) s
> > ...+>
> >
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Felipe Balbi <balbi@kernel.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > After the recent discussion about a patch which tried to add a check
> > against NULL after container_of(), I realized that there are a number
> > of such checks in the kernel.
> >
> > Now the big question: Are patches like this acceptable, or do they count
> > as noise ?
> 
> Not noise in my book :-)
> 
> Acked-by: Felipe Balbi <balbi@kernel.org>

Likewise,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

And thank you for the patch.

-- 
Regards,

Laurent Pinchart