Re: [PATCH v2] usb: atm: cxacru: Merge cxacru_upload_firmware() into cxacru_heavy_init()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Jul 22, 2025 at 12:11:18PM -0700, Nathan Chancellor wrote:
> After a recent change in clang to expose uninitialized warnings from
> const variables [1], there is a warning in cxacru_heavy_init():
> 
>   drivers/usb/atm/cxacru.c:1104:6: error: variable 'bp' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>    1104 |         if (instance->modem_type->boot_rom_patch) {
>         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/usb/atm/cxacru.c:1113:39: note: uninitialized use occurs here
>    1113 |         cxacru_upload_firmware(instance, fw, bp);
>         |                                              ^~
>   drivers/usb/atm/cxacru.c:1104:2: note: remove the 'if' if its condition is always true
>    1104 |         if (instance->modem_type->boot_rom_patch) {
>         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   drivers/usb/atm/cxacru.c:1095:32: note: initialize the variable 'bp' to silence this warning
>    1095 |         const struct firmware *fw, *bp;
>         |                                       ^
>         |                                        = NULL
> 
> While the warning is technically correct that bp is conditionally passed
> uninitialized to cxacru_upload_firmware(), it is ultimately a false
> positive warning on the uninitialized use of bp because the same
> condition that initializes bp, instance->modem_type->boot_rom_patch, is
> the same one that gates the use of bp within cxacru_upload_firmware().
> As this warning occurs in clang's frontend before inlining occurs, it
> cannot know that these conditions are indentical to avoid the warning.
> 
> Manually inline cxacru_upload_firmware() into cxacru_heavy_init(), as
> that is its only callsite, so that clang can see that bp is initialized
> and used under the same condition, clearing up the warning without any
> functional changes to the code (LLVM was already doing this inlining
> later).
> 
> Cc: stable@vger.kernel.org
> Fixes: 1b0e61465234 ("[PATCH] USB ATM: driver for the Conexant AccessRunner chipset cxacru")
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2102
> Link: https://github.com/llvm/llvm-project/commit/2464313eef01c5b1edf0eccf57a32cdee01472c7 [1]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Changes in v2:
> - Rather than initialize bp to NULL, manually inline
>   cxacru_upload_firmware() into cxacru_heavy_init() so that clang can
>   see the matching conditions for bp's initialization and use (based on
>   feedback from Greg).
> - Drop accessrunner-general@lists.sourceforge.net, as I got bounces when
>   sending to it unsubscribed.
> - Link to v1: https://lore.kernel.org/r/20250715-usb-cxacru-fix-clang-21-uninit-warning-v1-1-de6c652c3079@kernel.org
> ---
>  drivers/usb/atm/cxacru.c | 106 ++++++++++++++++++++++-------------------------
>  1 file changed, 49 insertions(+), 57 deletions(-)

Sorry for the churn, but hey, it's less code now!

Nice work :)

greg k-h