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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <70565b52-d42d-4808-84cd-ac8587c75b10@gmail.com>
Date: Thu, 27 Nov 2025 15:35:37 -0500
From: David Hunter <david.hunter.linux@...il.com>
To: Clint George <clintbgeorge@...il.com>, stern@...land.harvard.edu,
 gregkh@...uxfoundation.org
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org,
 khalid@...nel.org
Subject: Re: [PATCH 0/8] usb: gadget: dummy_hcd: coding style improvements and
 error handling change

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