* [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors @ 2025-12-03 8:33 hariconscious 2025-12-03 8:56 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: hariconscious @ 2025-12-03 8:33 UTC (permalink / raw) To: perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux Cc: khalid, shuah, david.hunter.linux, linux-sound, linux-kernel, HariKrishna Sagala From: HariKrishna Sagala <hariconscious@gmail.com> Initialize 'status1' with a default value to resolve the static analysis smatch reported error "uninitialized symbol 'status1'". The 'status1' variable is used to create a buff using "kmemdup". So, ensure to initialize the value before it is read. Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> --- This patch fixes the below smatch reported errors. sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. The below is the flow of 'status1' it is used before initialization. snd_rme_rate_get -> status1 is uninitialized and passed snd_rme_get_status1 -> passed as is snd_rme_read_value -> passed as is snd_usb_ctl_msg -> created buf from status1 using kmemdup usb_control_msg -> sent buf for reading/writing Description of "usb_control_msg", states as " * @data: pointer to the data to send" Later from Usb control request, dst buf is copied to src buf but usb control msg request is made before initialization. Thank you. sound/usb/mixer_quirks.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 828af3095b86..06903c5de087 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - u32 status1; + u32 status1 = 0; u32 rate = 0; int idx; int err; @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - u32 status1; + u32 status1 = 0; int idx = SND_RME_CLOCK_NOLOCK; int err; @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - u32 status1; + u32 status1 = 0; int err; err = snd_rme_get_status1(kcontrol, &status1); @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - u32 status1; + u32 status1 = 0; int err; err = snd_rme_get_status1(kcontrol, &status1); @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - u32 status1; + u32 status1 = 0; int err; err = snd_rme_get_status1(kcontrol, &status1); base-commit: 4a26e7032d7d57c998598c08a034872d6f0d3945 -- 2.43.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 8:33 [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors hariconscious @ 2025-12-03 8:56 ` Takashi Iwai 2025-12-03 11:01 ` HariKrishna Sagala 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2025-12-03 8:56 UTC (permalink / raw) To: hariconscious Cc: perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 03 Dec 2025 09:33:20 +0100, hariconscious@gmail.com wrote: > > From: HariKrishna Sagala <hariconscious@gmail.com> > > Initialize 'status1' with a default value to resolve the static analysis > smatch reported error "uninitialized symbol 'status1'". > The 'status1' variable is used to create a buff using "kmemdup". > So, ensure to initialize the value before it is read. > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > --- > This patch fixes the below smatch reported errors. > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > The below is the flow of 'status1' it is used before initialization. > > snd_rme_rate_get -> status1 is uninitialized and passed > snd_rme_get_status1 -> passed as is > snd_rme_read_value -> passed as is > snd_usb_ctl_msg -> created buf from status1 using kmemdup > usb_control_msg -> sent buf for reading/writing > > Description of "usb_control_msg", states as > " * @data: pointer to the data to send" > > Later from Usb control request, dst buf is copied to src buf but usb > control msg request is made before initialization. > > Thank you. > > sound/usb/mixer_quirks.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > index 828af3095b86..06903c5de087 100644 > --- a/sound/usb/mixer_quirks.c > +++ b/sound/usb/mixer_quirks.c > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > - u32 status1; > + u32 status1 = 0; > u32 rate = 0; > int idx; > int err; > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > - u32 status1; > + u32 status1 = 0; > int idx = SND_RME_CLOCK_NOLOCK; > int err; > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > - u32 status1; > + u32 status1 = 0; > int err; > > err = snd_rme_get_status1(kcontrol, &status1); > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > - u32 status1; > + u32 status1 = 0; > int err; > > err = snd_rme_get_status1(kcontrol, &status1); > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > struct snd_ctl_elem_value *ucontrol) > { > - u32 status1; > + u32 status1 = 0; > int err; > > err = snd_rme_get_status1(kcontrol, &status1); > The warning itself is rather dubious. But it'd certainly give a safer feeling to cover the uninitialized variables, so it would still make some sense. But, the code change can be improved. e.g. initialize the value in the callee side, instead of callers; then it'll reduce all changes to a one-liner. At the next time, please look at the patterns you changed more closely and think again whether it's the best change or not before submission. On the second look, often you see things from a different perspective :) thanks, Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 8:56 ` Takashi Iwai @ 2025-12-03 11:01 ` HariKrishna Sagala 2025-12-03 11:09 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: HariKrishna Sagala @ 2025-12-03 11:01 UTC (permalink / raw) To: Takashi Iwai, hariconscious Cc: perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 3 Dec 2025, Takashi Iwai wrote: > On Wed, 03 Dec 2025 09:33:20 +0100, > hariconscious@gmail.com wrote: > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > Initialize 'status1' with a default value to resolve the static analysis > > smatch reported error "uninitialized symbol 'status1'". > > The 'status1' variable is used to create a buff using "kmemdup". > > So, ensure to initialize the value before it is read. > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > --- > > This patch fixes the below smatch reported errors. > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > The below is the flow of 'status1' it is used before initialization. > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > snd_rme_get_status1 -> passed as is > > snd_rme_read_value -> passed as is > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > usb_control_msg -> sent buf for reading/writing > > > > Description of "usb_control_msg", states as > > " * @data: pointer to the data to send" > > > > Later from Usb control request, dst buf is copied to src buf but usb > > control msg request is made before initialization. > > > > Thank you. > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > index 828af3095b86..06903c5de087 100644 > > --- a/sound/usb/mixer_quirks.c > > +++ b/sound/usb/mixer_quirks.c > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > struct snd_ctl_elem_value *ucontrol) > > { > > - u32 status1; > > + u32 status1 = 0; > > u32 rate = 0; > > int idx; > > int err; > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > struct snd_ctl_elem_value *ucontrol) > > { > > - u32 status1; > > + u32 status1 = 0; > > int idx = SND_RME_CLOCK_NOLOCK; > > int err; > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > struct snd_ctl_elem_value *ucontrol) > > { > > - u32 status1; > > + u32 status1 = 0; > > int err; > > > > err = snd_rme_get_status1(kcontrol, &status1); > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > struct snd_ctl_elem_value *ucontrol) > > { > > - u32 status1; > > + u32 status1 = 0; > > int err; > > > > err = snd_rme_get_status1(kcontrol, &status1); > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > struct snd_ctl_elem_value *ucontrol) > > { > > - u32 status1; > > + u32 status1 = 0; > > int err; > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > The warning itself is rather dubious. But it'd certainly give a safer > feeling to cover the uninitialized variables, so it would still make > some sense. > > But, the code change can be improved. e.g. initialize the value in > the callee side, instead of callers; then it'll reduce all changes to > a one-liner. > > At the next time, please look at the patterns you changed more closely > and think again whether it's the best change or not before submission. > On the second look, often you see things from a different perspective > :) > > > thanks, > > Takashi > Hi Takashi, Thank you for the feedback and complement. Yes, will check the pattern and avoid this going forward. As all the functions call "snd_rme_get_status1", initialized here but smatch didn't silence them. I prefer initializing a variable in caller side as it owns and aware of initialized value sent. Sorry, if I had misunderstood your suggestion. Thank you, HariKrishna. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 11:01 ` HariKrishna Sagala @ 2025-12-03 11:09 ` Takashi Iwai 2025-12-03 11:22 ` HariKrishna Sagala 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2025-12-03 11:09 UTC (permalink / raw) To: HariKrishna Sagala Cc: Takashi Iwai, perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 03 Dec 2025 12:01:35 +0100, HariKrishna Sagala wrote: > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > On Wed, 03 Dec 2025 09:33:20 +0100, > > hariconscious@gmail.com wrote: > > > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > Initialize 'status1' with a default value to resolve the static analysis > > > smatch reported error "uninitialized symbol 'status1'". > > > The 'status1' variable is used to create a buff using "kmemdup". > > > So, ensure to initialize the value before it is read. > > > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > > --- > > > This patch fixes the below smatch reported errors. > > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > > > The below is the flow of 'status1' it is used before initialization. > > > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > > snd_rme_get_status1 -> passed as is > > > snd_rme_read_value -> passed as is > > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > > usb_control_msg -> sent buf for reading/writing > > > > > > Description of "usb_control_msg", states as > > > " * @data: pointer to the data to send" > > > > > > Later from Usb control request, dst buf is copied to src buf but usb > > > control msg request is made before initialization. > > > > > > Thank you. > > > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > > index 828af3095b86..06903c5de087 100644 > > > --- a/sound/usb/mixer_quirks.c > > > +++ b/sound/usb/mixer_quirks.c > > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > struct snd_ctl_elem_value *ucontrol) > > > { > > > - u32 status1; > > > + u32 status1 = 0; > > > u32 rate = 0; > > > int idx; > > > int err; > > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > struct snd_ctl_elem_value *ucontrol) > > > { > > > - u32 status1; > > > + u32 status1 = 0; > > > int idx = SND_RME_CLOCK_NOLOCK; > > > int err; > > > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > struct snd_ctl_elem_value *ucontrol) > > > { > > > - u32 status1; > > > + u32 status1 = 0; > > > int err; > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > struct snd_ctl_elem_value *ucontrol) > > > { > > > - u32 status1; > > > + u32 status1 = 0; > > > int err; > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > > struct snd_ctl_elem_value *ucontrol) > > > { > > > - u32 status1; > > > + u32 status1 = 0; > > > int err; > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > The warning itself is rather dubious. But it'd certainly give a safer > > feeling to cover the uninitialized variables, so it would still make > > some sense. > > > > But, the code change can be improved. e.g. initialize the value in > > the callee side, instead of callers; then it'll reduce all changes to > > a one-liner. > > > > At the next time, please look at the patterns you changed more closely > > and think again whether it's the best change or not before submission. > > On the second look, often you see things from a different perspective > > :) > > > > > > thanks, > > > > Takashi > > > > Hi Takashi, > > Thank you for the feedback and complement. > Yes, will check the pattern and avoid this going forward. > As all the functions call "snd_rme_get_status1", > initialized here but smatch didn't silence them. > > I prefer initializing a variable in caller side as it owns > and aware of initialized value sent. > > Sorry, if I had misunderstood your suggestion. Well, you seem driven too much by smatch. Just look at your patch again. You're repeating the same initializations and passing to the same function. And those are the only callers of snd_rme_get_status1(). So, instead of initializing in the caller side, initialize the value in snd_rme_get_status1(). Then the whole your changes will be oneliner like below. Takashi --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -2548,6 +2548,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, CLASS(snd_usb_lock, pm)(chip); if (pm.err < 0) return pm.err; + *status1 = 0; return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 11:09 ` Takashi Iwai @ 2025-12-03 11:22 ` HariKrishna Sagala 2025-12-03 11:50 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: HariKrishna Sagala @ 2025-12-03 11:22 UTC (permalink / raw) To: Takashi Iwai, HariKrishna Sagala Cc: perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 3 Dec 2025, Takashi Iwai wrote: > On Wed, 03 Dec 2025 12:01:35 +0100, > HariKrishna Sagala wrote: > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > On Wed, 03 Dec 2025 09:33:20 +0100, > > > hariconscious@gmail.com wrote: > > > > > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > > Initialize 'status1' with a default value to resolve the static analysis > > > > smatch reported error "uninitialized symbol 'status1'". > > > > The 'status1' variable is used to create a buff using "kmemdup". > > > > So, ensure to initialize the value before it is read. > > > > > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > > > --- > > > > This patch fixes the below smatch reported errors. > > > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > > > > > The below is the flow of 'status1' it is used before initialization. > > > > > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > > > snd_rme_get_status1 -> passed as is > > > > snd_rme_read_value -> passed as is > > > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > > > usb_control_msg -> sent buf for reading/writing > > > > > > > > Description of "usb_control_msg", states as > > > > " * @data: pointer to the data to send" > > > > > > > > Later from Usb control request, dst buf is copied to src buf but usb > > > > control msg request is made before initialization. > > > > > > > > Thank you. > > > > > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > > > index 828af3095b86..06903c5de087 100644 > > > > --- a/sound/usb/mixer_quirks.c > > > > +++ b/sound/usb/mixer_quirks.c > > > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > struct snd_ctl_elem_value *ucontrol) > > > > { > > > > - u32 status1; > > > > + u32 status1 = 0; > > > > u32 rate = 0; > > > > int idx; > > > > int err; > > > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > struct snd_ctl_elem_value *ucontrol) > > > > { > > > > - u32 status1; > > > > + u32 status1 = 0; > > > > int idx = SND_RME_CLOCK_NOLOCK; > > > > int err; > > > > > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > struct snd_ctl_elem_value *ucontrol) > > > > { > > > > - u32 status1; > > > > + u32 status1 = 0; > > > > int err; > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > struct snd_ctl_elem_value *ucontrol) > > > > { > > > > - u32 status1; > > > > + u32 status1 = 0; > > > > int err; > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > > > struct snd_ctl_elem_value *ucontrol) > > > > { > > > > - u32 status1; > > > > + u32 status1 = 0; > > > > int err; > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > > > The warning itself is rather dubious. But it'd certainly give a safer > > > feeling to cover the uninitialized variables, so it would still make > > > some sense. > > > > > > But, the code change can be improved. e.g. initialize the value in > > > the callee side, instead of callers; then it'll reduce all changes to > > > a one-liner. > > > > > > At the next time, please look at the patterns you changed more closely > > > and think again whether it's the best change or not before submission. > > > On the second look, often you see things from a different perspective > > > :) > > > > > > > > > thanks, > > > > > > Takashi > > > > > > > Hi Takashi, > > > > Thank you for the feedback and complement. > > Yes, will check the pattern and avoid this going forward. > > As all the functions call "snd_rme_get_status1", > > initialized here but smatch didn't silence them. > > > > I prefer initializing a variable in caller side as it owns > > and aware of initialized value sent. > > > > Sorry, if I had misunderstood your suggestion. > > Well, you seem driven too much by smatch. Just look at your patch > again. > > You're repeating the same initializations and passing to the same > function. And those are the only callers of snd_rme_get_status1(). > So, instead of initializing in the caller side, initialize the value > in snd_rme_get_status1(). Then the whole your changes will be > oneliner like below. > > > Takashi > > --- a/sound/usb/mixer_quirks.c > +++ b/sound/usb/mixer_quirks.c > @@ -2548,6 +2548,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > CLASS(snd_usb_lock, pm)(chip); > if (pm.err < 0) > return pm.err; > + *status1 = 0; > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > } > > Hi Takashi, Yes, tried that before, smatch didn't silence them. If it is worth having this change though they are not silenced, will raise v2 otherwise will drop this patch. Please let me know. Thank you, HariKrishna. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 11:22 ` HariKrishna Sagala @ 2025-12-03 11:50 ` Takashi Iwai 2025-12-03 12:01 ` HariKrishna Sagala 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2025-12-03 11:50 UTC (permalink / raw) To: HariKrishna Sagala Cc: Takashi Iwai, perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 03 Dec 2025 12:22:27 +0100, HariKrishna Sagala wrote: > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > On Wed, 03 Dec 2025 12:01:35 +0100, > > HariKrishna Sagala wrote: > > > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > > > On Wed, 03 Dec 2025 09:33:20 +0100, > > > > hariconscious@gmail.com wrote: > > > > > > > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > > > > Initialize 'status1' with a default value to resolve the static analysis > > > > > smatch reported error "uninitialized symbol 'status1'". > > > > > The 'status1' variable is used to create a buff using "kmemdup". > > > > > So, ensure to initialize the value before it is read. > > > > > > > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > > > > --- > > > > > This patch fixes the below smatch reported errors. > > > > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > > > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > > > > > > > The below is the flow of 'status1' it is used before initialization. > > > > > > > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > > > > snd_rme_get_status1 -> passed as is > > > > > snd_rme_read_value -> passed as is > > > > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > > > > usb_control_msg -> sent buf for reading/writing > > > > > > > > > > Description of "usb_control_msg", states as > > > > > " * @data: pointer to the data to send" > > > > > > > > > > Later from Usb control request, dst buf is copied to src buf but usb > > > > > control msg request is made before initialization. > > > > > > > > > > Thank you. > > > > > > > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > > > > index 828af3095b86..06903c5de087 100644 > > > > > --- a/sound/usb/mixer_quirks.c > > > > > +++ b/sound/usb/mixer_quirks.c > > > > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > { > > > > > - u32 status1; > > > > > + u32 status1 = 0; > > > > > u32 rate = 0; > > > > > int idx; > > > > > int err; > > > > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > { > > > > > - u32 status1; > > > > > + u32 status1 = 0; > > > > > int idx = SND_RME_CLOCK_NOLOCK; > > > > > int err; > > > > > > > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > { > > > > > - u32 status1; > > > > > + u32 status1 = 0; > > > > > int err; > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > { > > > > > - u32 status1; > > > > > + u32 status1 = 0; > > > > > int err; > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > { > > > > > - u32 status1; > > > > > + u32 status1 = 0; > > > > > int err; > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > > > > > > The warning itself is rather dubious. But it'd certainly give a safer > > > > feeling to cover the uninitialized variables, so it would still make > > > > some sense. > > > > > > > > But, the code change can be improved. e.g. initialize the value in > > > > the callee side, instead of callers; then it'll reduce all changes to > > > > a one-liner. > > > > > > > > At the next time, please look at the patterns you changed more closely > > > > and think again whether it's the best change or not before submission. > > > > On the second look, often you see things from a different perspective > > > > :) > > > > > > > > > > > > thanks, > > > > > > > > Takashi > > > > > > > > > > Hi Takashi, > > > > > > Thank you for the feedback and complement. > > > Yes, will check the pattern and avoid this going forward. > > > As all the functions call "snd_rme_get_status1", > > > initialized here but smatch didn't silence them. > > > > > > I prefer initializing a variable in caller side as it owns > > > and aware of initialized value sent. > > > > > > Sorry, if I had misunderstood your suggestion. > > > > Well, you seem driven too much by smatch. Just look at your patch > > again. > > > > You're repeating the same initializations and passing to the same > > function. And those are the only callers of snd_rme_get_status1(). > > So, instead of initializing in the caller side, initialize the value > > in snd_rme_get_status1(). Then the whole your changes will be > > oneliner like below. > > > > > > Takashi > > > > --- a/sound/usb/mixer_quirks.c > > +++ b/sound/usb/mixer_quirks.c > > @@ -2548,6 +2548,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > CLASS(snd_usb_lock, pm)(chip); > > if (pm.err < 0) > > return pm.err; > > + *status1 = 0; > > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > > } > > > > > > Hi Takashi, > > Yes, tried that before, smatch didn't silence them. Then it's an obvious failure of smatch. Or you might set *status1 = 0 at the beginning of snd_rme_get_status1() although it makes little sense. > If it is worth having this change though they are not silenced, > will raise v2 otherwise will drop this patch. If the smatch still complains, it's not worth. Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 11:50 ` Takashi Iwai @ 2025-12-03 12:01 ` HariKrishna Sagala 2025-12-03 12:54 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: HariKrishna Sagala @ 2025-12-03 12:01 UTC (permalink / raw) To: Takashi Iwai, HariKrishna Sagala Cc: perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 3 Dec 2025, Takashi Iwai wrote: > On Wed, 03 Dec 2025 12:22:27 +0100, > HariKrishna Sagala wrote: > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > On Wed, 03 Dec 2025 12:01:35 +0100, > > > HariKrishna Sagala wrote: > > > > > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > > > > > On Wed, 03 Dec 2025 09:33:20 +0100, > > > > > hariconscious@gmail.com wrote: > > > > > > > > > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > > > > > > Initialize 'status1' with a default value to resolve the static analysis > > > > > > smatch reported error "uninitialized symbol 'status1'". > > > > > > The 'status1' variable is used to create a buff using "kmemdup". > > > > > > So, ensure to initialize the value before it is read. > > > > > > > > > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > --- > > > > > > This patch fixes the below smatch reported errors. > > > > > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > > > > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > > > > > > > > > The below is the flow of 'status1' it is used before initialization. > > > > > > > > > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > > > > > snd_rme_get_status1 -> passed as is > > > > > > snd_rme_read_value -> passed as is > > > > > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > > > > > usb_control_msg -> sent buf for reading/writing > > > > > > > > > > > > Description of "usb_control_msg", states as > > > > > > " * @data: pointer to the data to send" > > > > > > > > > > > > Later from Usb control request, dst buf is copied to src buf but usb > > > > > > control msg request is made before initialization. > > > > > > > > > > > > Thank you. > > > > > > > > > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > > > > > index 828af3095b86..06903c5de087 100644 > > > > > > --- a/sound/usb/mixer_quirks.c > > > > > > +++ b/sound/usb/mixer_quirks.c > > > > > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > { > > > > > > - u32 status1; > > > > > > + u32 status1 = 0; > > > > > > u32 rate = 0; > > > > > > int idx; > > > > > > int err; > > > > > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > { > > > > > > - u32 status1; > > > > > > + u32 status1 = 0; > > > > > > int idx = SND_RME_CLOCK_NOLOCK; > > > > > > int err; > > > > > > > > > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > { > > > > > > - u32 status1; > > > > > > + u32 status1 = 0; > > > > > > int err; > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > { > > > > > > - u32 status1; > > > > > > + u32 status1 = 0; > > > > > > int err; > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > { > > > > > > - u32 status1; > > > > > > + u32 status1 = 0; > > > > > > int err; > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > > > > > > > > > The warning itself is rather dubious. But it'd certainly give a safer > > > > > feeling to cover the uninitialized variables, so it would still make > > > > > some sense. > > > > > > > > > > But, the code change can be improved. e.g. initialize the value in > > > > > the callee side, instead of callers; then it'll reduce all changes to > > > > > a one-liner. > > > > > > > > > > At the next time, please look at the patterns you changed more closely > > > > > and think again whether it's the best change or not before submission. > > > > > On the second look, often you see things from a different perspective > > > > > :) > > > > > > > > > > > > > > > thanks, > > > > > > > > > > Takashi > > > > > > > > > > > > > Hi Takashi, > > > > > > > > Thank you for the feedback and complement. > > > > Yes, will check the pattern and avoid this going forward. > > > > As all the functions call "snd_rme_get_status1", > > > > initialized here but smatch didn't silence them. > > > > > > > > I prefer initializing a variable in caller side as it owns > > > > and aware of initialized value sent. > > > > > > > > Sorry, if I had misunderstood your suggestion. > > > > > > Well, you seem driven too much by smatch. Just look at your patch > > > again. > > > > > > You're repeating the same initializations and passing to the same > > > function. And those are the only callers of snd_rme_get_status1(). > > > So, instead of initializing in the caller side, initialize the value > > > in snd_rme_get_status1(). Then the whole your changes will be > > > oneliner like below. > > > > > > > > > Takashi > > > > > > --- a/sound/usb/mixer_quirks.c > > > +++ b/sound/usb/mixer_quirks.c > > > @@ -2548,6 +2548,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > CLASS(snd_usb_lock, pm)(chip); > > > if (pm.err < 0) > > > return pm.err; > > > + *status1 = 0; > > > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > > > } > > > > > > > > > > Hi Takashi, > > > > Yes, tried that before, smatch didn't silence them. > > Then it's an obvious failure of smatch. > > Or you might set *status1 = 0 at the beginning of > snd_rme_get_status1() although it makes little sense. > > > If it is worth having this change though they are not silenced, > > will raise v2 otherwise will drop this patch. > > If the smatch still complains, it's not worth. > > > Takashi > Hi Takashi, This is the change I tried but smatch didn't silence them. diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 828af3095b86..10354576d325 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -2443,6 +2443,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, CLASS(snd_usb_lock, pm)(chip); if (pm.err < 0) return pm.err; + *status1 = 0; return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); } Below are reported with the change, as seen they are not silenced. sound/usb/mixer_quirks.c:2463 snd_rme_rate_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2468 snd_rme_rate_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2473 snd_rme_rate_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2496 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2502 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2523 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2536 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:2549 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. sound/usb/mixer_quirks.c:4137 snd_djm_controls_info() warn: potential spectre issue 'ctl->options' [r] (local cap) Sorry not sure about the reason. As they are not removed, dropping this patch. Thank you for the support. ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 12:01 ` HariKrishna Sagala @ 2025-12-03 12:54 ` Takashi Iwai 2025-12-03 13:34 ` HariKrishna Sagala 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2025-12-03 12:54 UTC (permalink / raw) To: HariKrishna Sagala Cc: Takashi Iwai, perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 03 Dec 2025 13:01:48 +0100, HariKrishna Sagala wrote: > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > On Wed, 03 Dec 2025 12:22:27 +0100, > > HariKrishna Sagala wrote: > > > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > > > On Wed, 03 Dec 2025 12:01:35 +0100, > > > > HariKrishna Sagala wrote: > > > > > > > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > > > > > > > On Wed, 03 Dec 2025 09:33:20 +0100, > > > > > > hariconscious@gmail.com wrote: > > > > > > > > > > > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > > > > > > > > Initialize 'status1' with a default value to resolve the static analysis > > > > > > > smatch reported error "uninitialized symbol 'status1'". > > > > > > > The 'status1' variable is used to create a buff using "kmemdup". > > > > > > > So, ensure to initialize the value before it is read. > > > > > > > > > > > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > --- > > > > > > > This patch fixes the below smatch reported errors. > > > > > > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > > > > > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > > > > > > > > > > > The below is the flow of 'status1' it is used before initialization. > > > > > > > > > > > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > > > > > > snd_rme_get_status1 -> passed as is > > > > > > > snd_rme_read_value -> passed as is > > > > > > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > > > > > > usb_control_msg -> sent buf for reading/writing > > > > > > > > > > > > > > Description of "usb_control_msg", states as > > > > > > > " * @data: pointer to the data to send" > > > > > > > > > > > > > > Later from Usb control request, dst buf is copied to src buf but usb > > > > > > > control msg request is made before initialization. > > > > > > > > > > > > > > Thank you. > > > > > > > > > > > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > > > > > > index 828af3095b86..06903c5de087 100644 > > > > > > > --- a/sound/usb/mixer_quirks.c > > > > > > > +++ b/sound/usb/mixer_quirks.c > > > > > > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > > > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > { > > > > > > > - u32 status1; > > > > > > > + u32 status1 = 0; > > > > > > > u32 rate = 0; > > > > > > > int idx; > > > > > > > int err; > > > > > > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > { > > > > > > > - u32 status1; > > > > > > > + u32 status1 = 0; > > > > > > > int idx = SND_RME_CLOCK_NOLOCK; > > > > > > > int err; > > > > > > > > > > > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > { > > > > > > > - u32 status1; > > > > > > > + u32 status1 = 0; > > > > > > > int err; > > > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > { > > > > > > > - u32 status1; > > > > > > > + u32 status1 = 0; > > > > > > > int err; > > > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > { > > > > > > > - u32 status1; > > > > > > > + u32 status1 = 0; > > > > > > > int err; > > > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > > > > > > > > > > > > The warning itself is rather dubious. But it'd certainly give a safer > > > > > > feeling to cover the uninitialized variables, so it would still make > > > > > > some sense. > > > > > > > > > > > > But, the code change can be improved. e.g. initialize the value in > > > > > > the callee side, instead of callers; then it'll reduce all changes to > > > > > > a one-liner. > > > > > > > > > > > > At the next time, please look at the patterns you changed more closely > > > > > > and think again whether it's the best change or not before submission. > > > > > > On the second look, often you see things from a different perspective > > > > > > :) > > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > > > > Takashi > > > > > > > > > > > > > > > > Hi Takashi, > > > > > > > > > > Thank you for the feedback and complement. > > > > > Yes, will check the pattern and avoid this going forward. > > > > > As all the functions call "snd_rme_get_status1", > > > > > initialized here but smatch didn't silence them. > > > > > > > > > > I prefer initializing a variable in caller side as it owns > > > > > and aware of initialized value sent. > > > > > > > > > > Sorry, if I had misunderstood your suggestion. > > > > > > > > Well, you seem driven too much by smatch. Just look at your patch > > > > again. > > > > > > > > You're repeating the same initializations and passing to the same > > > > function. And those are the only callers of snd_rme_get_status1(). > > > > So, instead of initializing in the caller side, initialize the value > > > > in snd_rme_get_status1(). Then the whole your changes will be > > > > oneliner like below. > > > > > > > > > > > > Takashi > > > > > > > > --- a/sound/usb/mixer_quirks.c > > > > +++ b/sound/usb/mixer_quirks.c > > > > @@ -2548,6 +2548,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > CLASS(snd_usb_lock, pm)(chip); > > > > if (pm.err < 0) > > > > return pm.err; > > > > + *status1 = 0; > > > > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > > > > } > > > > > > > > > > > > > > Hi Takashi, > > > > > > Yes, tried that before, smatch didn't silence them. > > > > Then it's an obvious failure of smatch. > > > > Or you might set *status1 = 0 at the beginning of > > snd_rme_get_status1() although it makes little sense. > > > > > If it is worth having this change though they are not silenced, > > > will raise v2 otherwise will drop this patch. > > > > If the smatch still complains, it's not worth. > > > > > > Takashi > > > > Hi Takashi, > > This is the change I tried but smatch didn't silence them. > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > index 828af3095b86..10354576d325 100644 > --- a/sound/usb/mixer_quirks.c > +++ b/sound/usb/mixer_quirks.c > @@ -2443,6 +2443,7 @@ static int snd_rme_get_status1(struct snd_kcontrol > *kcontrol, > CLASS(snd_usb_lock, pm)(chip); > if (pm.err < 0) > return pm.err; > + *status1 = 0; > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > } > > Below are reported with the change, as seen they are not silenced. > > sound/usb/mixer_quirks.c:2463 snd_rme_rate_get() error: uninitialized > symbol 'status1'. > sound/usb/mixer_quirks.c:2468 snd_rme_rate_get() error: uninitialized > symbol 'status1'. > sound/usb/mixer_quirks.c:2473 snd_rme_rate_get() error: uninitialized > symbol 'status1'. > sound/usb/mixer_quirks.c:2496 snd_rme_sync_state_get() error: > uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2502 snd_rme_sync_state_get() error: > uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2523 snd_rme_spdif_if_get() error: uninitialized > symbol 'status1'. > sound/usb/mixer_quirks.c:2536 snd_rme_spdif_format_get() error: > uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:2549 snd_rme_sync_source_get() error: > uninitialized symbol 'status1'. > sound/usb/mixer_quirks.c:4137 snd_djm_controls_info() warn: potential > spectre issue 'ctl->options' [r] (local cap) As mentioned, try to put *status1=0 line before CLASS() call in the above. If this still doesn't help, it's a bug of smatch, and we should ignore it. Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors 2025-12-03 12:54 ` Takashi Iwai @ 2025-12-03 13:34 ` HariKrishna Sagala 0 siblings, 0 replies; 9+ messages in thread From: HariKrishna Sagala @ 2025-12-03 13:34 UTC (permalink / raw) To: Takashi Iwai, HariKrishna Sagala Cc: perex, tiwai, cristian.ciocaltea, cryolitia, franta-linux, khalid, shuah, david.hunter.linux, linux-sound, linux-kernel On Wed, 3 Dec 2025, Takashi Iwai wrote: > On Wed, 03 Dec 2025 13:01:48 +0100, > HariKrishna Sagala wrote: > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > On Wed, 03 Dec 2025 12:22:27 +0100, > > > HariKrishna Sagala wrote: > > > > > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > > > > > On Wed, 03 Dec 2025 12:01:35 +0100, > > > > > HariKrishna Sagala wrote: > > > > > > > > > > > > On Wed, 3 Dec 2025, Takashi Iwai wrote: > > > > > > > > > > > > > On Wed, 03 Dec 2025 09:33:20 +0100, > > > > > > > hariconscious@gmail.com wrote: > > > > > > > > > > > > > > > > From: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > > > > > > > > > > Initialize 'status1' with a default value to resolve the static analysis > > > > > > > > smatch reported error "uninitialized symbol 'status1'". > > > > > > > > The 'status1' variable is used to create a buff using "kmemdup". > > > > > > > > So, ensure to initialize the value before it is read. > > > > > > > > > > > > > > > > Signed-off-by: HariKrishna Sagala <hariconscious@gmail.com> > > > > > > > > --- > > > > > > > > This patch fixes the below smatch reported errors. > > > > > > > > sound/usb/mixer_quirks.c:2462 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2467 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2472 snd_rme_rate_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2495 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2501 snd_rme_sync_state_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2522 snd_rme_spdif_if_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2535 snd_rme_spdif_format_get() error: uninitialized symbol 'status1'. > > > > > > > > sound/usb/mixer_quirks.c:2548 snd_rme_sync_source_get() error: uninitialized symbol 'status1'. > > > > > > > > > > > > > > > > The below is the flow of 'status1' it is used before initialization. > > > > > > > > > > > > > > > > snd_rme_rate_get -> status1 is uninitialized and passed > > > > > > > > snd_rme_get_status1 -> passed as is > > > > > > > > snd_rme_read_value -> passed as is > > > > > > > > snd_usb_ctl_msg -> created buf from status1 using kmemdup > > > > > > > > usb_control_msg -> sent buf for reading/writing > > > > > > > > > > > > > > > > Description of "usb_control_msg", states as > > > > > > > > " * @data: pointer to the data to send" > > > > > > > > > > > > > > > > Later from Usb control request, dst buf is copied to src buf but usb > > > > > > > > control msg request is made before initialization. > > > > > > > > > > > > > > > > Thank you. > > > > > > > > > > > > > > > > sound/usb/mixer_quirks.c | 10 +++++----- > > > > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > > > > > > > index 828af3095b86..06903c5de087 100644 > > > > > > > > --- a/sound/usb/mixer_quirks.c > > > > > > > > +++ b/sound/usb/mixer_quirks.c > > > > > > > > @@ -2449,7 +2449,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > > > > > static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > > { > > > > > > > > - u32 status1; > > > > > > > > + u32 status1 = 0; > > > > > > > > u32 rate = 0; > > > > > > > > int idx; > > > > > > > > int err; > > > > > > > > @@ -2483,7 +2483,7 @@ static int snd_rme_rate_get(struct snd_kcontrol *kcontrol, > > > > > > > > static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > > { > > > > > > > > - u32 status1; > > > > > > > > + u32 status1 = 0; > > > > > > > > int idx = SND_RME_CLOCK_NOLOCK; > > > > > > > > int err; > > > > > > > > > > > > > > > > @@ -2513,7 +2513,7 @@ static int snd_rme_sync_state_get(struct snd_kcontrol *kcontrol, > > > > > > > > static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > > { > > > > > > > > - u32 status1; > > > > > > > > + u32 status1 = 0; > > > > > > > > int err; > > > > > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > @@ -2526,7 +2526,7 @@ static int snd_rme_spdif_if_get(struct snd_kcontrol *kcontrol, > > > > > > > > static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > > { > > > > > > > > - u32 status1; > > > > > > > > + u32 status1 = 0; > > > > > > > > int err; > > > > > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > @@ -2539,7 +2539,7 @@ static int snd_rme_spdif_format_get(struct snd_kcontrol *kcontrol, > > > > > > > > static int snd_rme_sync_source_get(struct snd_kcontrol *kcontrol, > > > > > > > > struct snd_ctl_elem_value *ucontrol) > > > > > > > > { > > > > > > > > - u32 status1; > > > > > > > > + u32 status1 = 0; > > > > > > > > int err; > > > > > > > > > > > > > > > > err = snd_rme_get_status1(kcontrol, &status1); > > > > > > > > > > > > > > > > > > > > > > The warning itself is rather dubious. But it'd certainly give a safer > > > > > > > feeling to cover the uninitialized variables, so it would still make > > > > > > > some sense. > > > > > > > > > > > > > > But, the code change can be improved. e.g. initialize the value in > > > > > > > the callee side, instead of callers; then it'll reduce all changes to > > > > > > > a one-liner. > > > > > > > > > > > > > > At the next time, please look at the patterns you changed more closely > > > > > > > and think again whether it's the best change or not before submission. > > > > > > > On the second look, often you see things from a different perspective > > > > > > > :) > > > > > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > > > > > > Takashi > > > > > > > > > > > > > > > > > > > Hi Takashi, > > > > > > > > > > > > Thank you for the feedback and complement. > > > > > > Yes, will check the pattern and avoid this going forward. > > > > > > As all the functions call "snd_rme_get_status1", > > > > > > initialized here but smatch didn't silence them. > > > > > > > > > > > > I prefer initializing a variable in caller side as it owns > > > > > > and aware of initialized value sent. > > > > > > > > > > > > Sorry, if I had misunderstood your suggestion. > > > > > > > > > > Well, you seem driven too much by smatch. Just look at your patch > > > > > again. > > > > > > > > > > You're repeating the same initializations and passing to the same > > > > > function. And those are the only callers of snd_rme_get_status1(). > > > > > So, instead of initializing in the caller side, initialize the value > > > > > in snd_rme_get_status1(). Then the whole your changes will be > > > > > oneliner like below. > > > > > > > > > > > > > > > Takashi > > > > > > > > > > --- a/sound/usb/mixer_quirks.c > > > > > +++ b/sound/usb/mixer_quirks.c > > > > > @@ -2548,6 +2548,7 @@ static int snd_rme_get_status1(struct snd_kcontrol *kcontrol, > > > > > CLASS(snd_usb_lock, pm)(chip); > > > > > if (pm.err < 0) > > > > > return pm.err; > > > > > + *status1 = 0; > > > > > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > > > > > } > > > > > > > > > > > > > > > > > > Hi Takashi, > > > > > > > > Yes, tried that before, smatch didn't silence them. > > > > > > Then it's an obvious failure of smatch. > > > > > > Or you might set *status1 = 0 at the beginning of > > > snd_rme_get_status1() although it makes little sense. > > > > > > > If it is worth having this change though they are not silenced, > > > > will raise v2 otherwise will drop this patch. > > > > > > If the smatch still complains, it's not worth. > > > > > > > > > Takashi > > > > > > > Hi Takashi, > > > > This is the change I tried but smatch didn't silence them. > > > > diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c > > index 828af3095b86..10354576d325 100644 > > --- a/sound/usb/mixer_quirks.c > > +++ b/sound/usb/mixer_quirks.c > > @@ -2443,6 +2443,7 @@ static int snd_rme_get_status1(struct snd_kcontrol > > *kcontrol, > > CLASS(snd_usb_lock, pm)(chip); > > if (pm.err < 0) > > return pm.err; > > + *status1 = 0; > > return snd_rme_read_value(chip, SND_RME_GET_STATUS1, status1); > > } > > > > Below are reported with the change, as seen they are not silenced. > > > > sound/usb/mixer_quirks.c:2463 snd_rme_rate_get() error: uninitialized > > symbol 'status1'. > > sound/usb/mixer_quirks.c:2468 snd_rme_rate_get() error: uninitialized > > symbol 'status1'. > > sound/usb/mixer_quirks.c:2473 snd_rme_rate_get() error: uninitialized > > symbol 'status1'. > > sound/usb/mixer_quirks.c:2496 snd_rme_sync_state_get() error: > > uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2502 snd_rme_sync_state_get() error: > > uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2523 snd_rme_spdif_if_get() error: uninitialized > > symbol 'status1'. > > sound/usb/mixer_quirks.c:2536 snd_rme_spdif_format_get() error: > > uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:2549 snd_rme_sync_source_get() error: > > uninitialized symbol 'status1'. > > sound/usb/mixer_quirks.c:4137 snd_djm_controls_info() warn: potential > > spectre issue 'ctl->options' [r] (local cap) > > As mentioned, try to put *status1=0 line before CLASS() call in the > above. If this still doesn't help, it's a bug of smatch, and we > should ignore it. > > > Takashi > Hi Takashi, Yes, with this change it got resolved :) Will raise v2 for the same. I need to understand the change before CLASS() is helping to resolve this. (Action item on me) Thank you for the support. HariKrishna. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-12-03 13:34 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-03 8:33 [PATCH] ALSA: usb-audio: Initialize status1 to fix uninitialized symbol errors hariconscious 2025-12-03 8:56 ` Takashi Iwai 2025-12-03 11:01 ` HariKrishna Sagala 2025-12-03 11:09 ` Takashi Iwai 2025-12-03 11:22 ` HariKrishna Sagala 2025-12-03 11:50 ` Takashi Iwai 2025-12-03 12:01 ` HariKrishna Sagala 2025-12-03 12:54 ` Takashi Iwai 2025-12-03 13:34 ` HariKrishna Sagala
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox