Re: [PATCH] sound: fix uninit-value in i2s_dma_isr
From: Suraj Sonawane
Date: Thu Oct 31 2024 - 02:48:22 EST
On 30/10/24 22:44, Mark Brown wrote:
On Wed, Oct 30, 2024 at 10:38:29PM +0530, Suraj Sonawane wrote:
Fix an issue detected by the Smatch tool:
sound/soc/bcm/bcm63xx-pcm-whistler.c:264 i2s_dma_isr()
error: uninitialized symbol 'val_1'.
sound/soc/bcm/bcm63xx-pcm-whistler.c:264 i2s_dma_isr()
error: uninitialized symbol 'val_2'.
These errors occurred because the variables 'val_1' and 'val_2' are
declared but may not be assigned a value before they are used.
Specifically, if the loop that assigns values to 'val_1' and 'val_2'
does not execute (for example, when 'offlevel' is zero), these
variables remain uninitialized, leading to potential undefined
behavior.
To resolve this issue, initialize 'val_1' and 'val_2' to 0 at the
point of declaration. This ensures that 'val_1' and 'val_2' have
defined values before they are used in subsequent calculations,
preventing any warnings or undefined behavior in cases where the
loop does not run.
This will shut the warning up, but why are these values valid? Are we
handling the cases where the loops do not execute properly?
Thank you for the feedback and your time.
The uninitialized warning for val_1 and val_2 arises because, in some
cases, the offlevel value is zero, and as a result, the loop does not
execute, leaving these variables potentially undefined. The subsequent
code calculates prtd->dma_addr_next using val_1 + val_2, so it's
necessary to have val_1 and val_2 initialized to a known value, even
when the loop does not run.
Initializing them to zero ensures prtd->dma_addr_next has a defined
value without triggering undefined behavior. However, if a zero
initialization could cause unintended behavior in dma_addr_next, I could
alternatively handle this case by setting dma_addr_next conditionally
when offlevel is non-zero.
Let me know if there’s a preferred approach, or if you'd suggest a
different initial value for these variables based on the expected use.