usb: gadget: dummy_hcd: coding style improvements and error handling change

[PATCH 0/8] usb: gadget: dummy_hcd: coding style improvements and error handling change

Posted by Clint George 1 week, 5 days ago

This patch series focuses on addressing various coding style issues in
the dummy_hcd USB gadget driver. The changes include simplifying error
handling by preventing kernel-space crashes, improving readability, and
ensuring consistency with kernel coding conventions.

Clint George (8):
  usb: gadget: dummy_hcd: replace BUG() with WARN_ON_ONCE()
  usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with
    octal (0444)
  usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'
  usb: gadget: dummy_hcd: fix block comments, blank lines and function
    braces
  usb: gadget: dummy_hcd: merge multi-line quoted strings into one line
  usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr
  usb: gadget: dummy_hcd: remove unnecessary 'else' after return
  usb: gadget: dummy_hcd: fix miscellaneous coding style warnings

 drivers/usb/gadget/udc/dummy_hcd.c | 139 ++++++++++++++---------------
 1 file changed, 67 insertions(+), 72 deletions(-)

-- 

Testing:
-  Verified dummy_hcd module loading and behavior by checking dmesg logs
for expected output.
- Ran compiler testing with no new warnings detected.
- Ensured the module builds and inserts cleanly without issues.
- Used Static Analysis tools to confirm no new issues were introduced.

Please review the changes and let me know if any modifications
or further testing of the module is required. 

Thanks,
Clint

2.43.0

Re: [PATCH 0/8] usb: gadget: dummy_hcd: coding style improvements and error handling change

Posted by David Hunter 4 days, 5 hours ago

On 11/19/25 08:08, Clint George wrote:
> This patch series focuses on addressing various coding style issues in
> the dummy_hcd USB gadget driver. The changes include simplifying error
> handling by preventing kernel-space crashes, improving readability, and
> ensuring consistency with kernel coding conventions.
> 
> Clint George (8):
>   usb: gadget: dummy_hcd: replace BUG() with WARN_ON_ONCE()

Hey Clint,

Regarding our discussion on Discord, I wanted to give you advice and
have it be closer to the code, so you could see what I was talking
about. You asked about Greg's feedback regarding the "Bug()". Here is
some context so that you can understand Greg's feedback a little better.

In Kernel development, there are thousands of people writing code. As a
result, developers will write something like "Bug()" or "Warn()" if a
particular path/condition is met.This is to create a signal for future
developers about situations that should not occur. A later developer
might do something that causes that faulty condition to be met. When
debugging, your goal is not to simply remove that line. Your goal is to
find out what caused the faulty condition, and fix that.

If all you do is eliminate the signal that there is an error, you are
just "papering over" instead of addressing the actual issue.

>   usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with
>     octal (0444)
>   usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'
>   usb: gadget: dummy_hcd: fix block comments, blank lines and function
>     braces

As we discussed, you can break your patches into something more focused
on one type of fix. For example, one patch could be to just remove code
and put in a more meaningful comment. Another patch could be removing
unnecessary spaces.

>   usb: gadget: dummy_hcd: merge multi-line quoted strings into one line
>   usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr
>   usb: gadget: dummy_hcd: remove unnecessary 'else' after return

Also, some things like your changing of the "else if" are things that
you do not need to change. Some of the knowledge of what to change and
what to ignore will come with experience. For that particular one, most
developers are used to seeing "else if".

>   usb: gadget: dummy_hcd: fix miscellaneous coding style warnings
> 
>  drivers/usb/gadget/udc/dummy_hcd.c | 139 ++++++++++++++---------------
>  1 file changed, 67 insertions(+), 72 deletions(-)
> 

Overall, my recommendation for you is to reduce the amount you are
trying to tackle at once. You can work over a longer period of time on
the file. Not everything needs to be accepted all at once.

Also, feel free to continue reaching out on discord. I just wrote here
to closer tie my feedback to particular patches.

Good attempt at submitting your first patch series, and don't get
discouraged. Getting feedback is a part of the process.

Thanks,
David Hunter

[PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements