* [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning
@ 2019-03-22 14:06 Arnd Bergmann
2019-03-22 14:53 ` Nathan Chancellor
2019-03-22 15:00 ` Takashi Iwai
0 siblings, 2 replies; 5+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:06 UTC (permalink / raw)
To: Takashi Iwai, Connor McAdams, Takashi Sakamoto
Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
Arnd Bergmann, Jaroslav Kysela, Alastair Bridgewater, alsa-devel,
linux-kernel
When CONFIG_PCI is disabled, clang gets confused about the
control flow of the switch() statement always ending up
in the default case, and warns:
sound/pci/hda/patch_ca0132.c:7558:6: error: variable 'fw_entry' is used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
if (!spec->alt_firmware_present) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/pci/hda/patch_ca0132.c:7565:42: note: uninitialized use occurs here
dsp_os_image = (struct dsp_image_seg *)(fw_entry->data);
^~~~~~~~
sound/pci/hda/patch_ca0132.c:7558:2: note: remove the 'if' if its condition is always true
if (!spec->alt_firmware_present) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/pci/hda/patch_ca0132.c:7521:33: note: initialize the variable 'fw_entry' to silence this warning
const struct firmware *fw_entry;
^
= NULL
Adding an explicit check for CONFIG_PCI avoids the issue.
Unfortunately this is not very intuitive here.
Link: https://bugs.llvm.org/show_bug.cgi?id=41197#c1
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Any suggestions for other workarounds appreciated. If you can think
of a better fix, please treat this as a reported-by:
---
sound/pci/hda/patch_ca0132.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index 29882bda7632..415b16b7db70 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -7555,7 +7555,7 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
* Use default ctefx.bin if no alt firmware is detected, or if none
* exists for your particular codec.
*/
- if (!spec->alt_firmware_present) {
+ if (!IS_ENABLED(CONFIG_PCI) || !spec->alt_firmware_present) {
codec_dbg(codec, "Default firmware selected.");
if (request_firmware(&fw_entry, EFX_FILE,
codec->card->dev) != 0)
--
2.20.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning
2019-03-22 14:06 [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning Arnd Bergmann
@ 2019-03-22 14:53 ` Nathan Chancellor
2019-03-22 15:00 ` Takashi Iwai
1 sibling, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2019-03-22 14:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Takashi Iwai, Connor McAdams, Takashi Sakamoto,
clang-built-linux, Nick Desaulniers, Jaroslav Kysela,
Alastair Bridgewater, alsa-devel, linux-kernel
On Fri, Mar 22, 2019 at 03:06:28PM +0100, Arnd Bergmann wrote:
> When CONFIG_PCI is disabled, clang gets confused about the
> control flow of the switch() statement always ending up
> in the default case, and warns:
>
> sound/pci/hda/patch_ca0132.c:7558:6: error: variable 'fw_entry' is used uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
> if (!spec->alt_firmware_present) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> sound/pci/hda/patch_ca0132.c:7565:42: note: uninitialized use occurs here
> dsp_os_image = (struct dsp_image_seg *)(fw_entry->data);
> ^~~~~~~~
> sound/pci/hda/patch_ca0132.c:7558:2: note: remove the 'if' if its condition is always true
> if (!spec->alt_firmware_present) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> sound/pci/hda/patch_ca0132.c:7521:33: note: initialize the variable 'fw_entry' to silence this warning
> const struct firmware *fw_entry;
> ^
> = NULL
>
> Adding an explicit check for CONFIG_PCI avoids the issue.
> Unfortunately this is not very intuitive here.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=41197#c1
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> Any suggestions for other workarounds appreciated. If you can think
> of a better fix, please treat this as a reported-by:
> ---
> sound/pci/hda/patch_ca0132.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
> index 29882bda7632..415b16b7db70 100644
> --- a/sound/pci/hda/patch_ca0132.c
> +++ b/sound/pci/hda/patch_ca0132.c
> @@ -7555,7 +7555,7 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
> * Use default ctefx.bin if no alt firmware is detected, or if none
> * exists for your particular codec.
> */
> - if (!spec->alt_firmware_present) {
> + if (!IS_ENABLED(CONFIG_PCI) || !spec->alt_firmware_present) {
> codec_dbg(codec, "Default firmware selected.");
> if (request_firmware(&fw_entry, EFX_FILE,
> codec->card->dev) != 0)
> --
> 2.20.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning
2019-03-22 14:06 [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning Arnd Bergmann
2019-03-22 14:53 ` Nathan Chancellor
@ 2019-03-22 15:00 ` Takashi Iwai
2019-03-22 15:12 ` Arnd Bergmann
2019-03-22 15:13 ` Nathan Chancellor
1 sibling, 2 replies; 5+ messages in thread
From: Takashi Iwai @ 2019-03-22 15:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Connor McAdams, Takashi Sakamoto, alsa-devel,
Alastair Bridgewater, Nathan Chancellor, Nick Desaulniers,
clang-built-linux, Jaroslav Kysela, linux-kernel
On Fri, 22 Mar 2019 15:06:28 +0100,
Arnd Bergmann wrote:
>
> When CONFIG_PCI is disabled, clang gets confused about the
> control flow of the switch() statement always ending up
> in the default case, and warns:
>
> sound/pci/hda/patch_ca0132.c:7558:6: error: variable 'fw_entry' is used uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
> if (!spec->alt_firmware_present) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> sound/pci/hda/patch_ca0132.c:7565:42: note: uninitialized use occurs here
> dsp_os_image = (struct dsp_image_seg *)(fw_entry->data);
> ^~~~~~~~
> sound/pci/hda/patch_ca0132.c:7558:2: note: remove the 'if' if its condition is always true
> if (!spec->alt_firmware_present) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> sound/pci/hda/patch_ca0132.c:7521:33: note: initialize the variable 'fw_entry' to silence this warning
> const struct firmware *fw_entry;
> ^
> = NULL
>
> Adding an explicit check for CONFIG_PCI avoids the issue.
> Unfortunately this is not very intuitive here.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=41197#c1
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Any suggestions for other workarounds appreciated. If you can think
> of a better fix, please treat this as a reported-by:
Can it be addressed by the code simplification like below, instead?
thanks,
Takashi
---
sound/pci/hda/patch_ca0132.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index 29882bda7632..e1ebc6d5f382 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -1005,7 +1005,6 @@ struct ca0132_spec {
unsigned int scp_resp_header;
unsigned int scp_resp_data[4];
unsigned int scp_resp_count;
- bool alt_firmware_present;
bool startup_check_entered;
bool dsp_reload;
@@ -7518,7 +7517,7 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
bool dsp_loaded = false;
struct ca0132_spec *spec = codec->spec;
const struct dsp_image_seg *dsp_os_image;
- const struct firmware *fw_entry;
+ const struct firmware *fw_entry = NULL;
/*
* Alternate firmwares for different variants. The Recon3Di apparently
* can use the default firmware, but I'll leave the option in case
@@ -7529,33 +7528,26 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
case QUIRK_R3D:
case QUIRK_AE5:
if (request_firmware(&fw_entry, DESKTOP_EFX_FILE,
- codec->card->dev) != 0) {
+ codec->card->dev) != 0)
codec_dbg(codec, "Desktop firmware not found.");
- spec->alt_firmware_present = false;
- } else {
+ else
codec_dbg(codec, "Desktop firmware selected.");
- spec->alt_firmware_present = true;
- }
break;
case QUIRK_R3DI:
if (request_firmware(&fw_entry, R3DI_EFX_FILE,
- codec->card->dev) != 0) {
+ codec->card->dev) != 0)
codec_dbg(codec, "Recon3Di alt firmware not detected.");
- spec->alt_firmware_present = false;
- } else {
+ else
codec_dbg(codec, "Recon3Di firmware selected.");
- spec->alt_firmware_present = true;
- }
break;
default:
- spec->alt_firmware_present = false;
break;
}
/*
* Use default ctefx.bin if no alt firmware is detected, or if none
* exists for your particular codec.
*/
- if (!spec->alt_firmware_present) {
+ if (!fw_entry) {
codec_dbg(codec, "Default firmware selected.");
if (request_firmware(&fw_entry, EFX_FILE,
codec->card->dev) != 0)
--
2.16.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning
2019-03-22 15:00 ` Takashi Iwai
@ 2019-03-22 15:12 ` Arnd Bergmann
2019-03-22 15:13 ` Nathan Chancellor
1 sibling, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2019-03-22 15:12 UTC (permalink / raw)
To: Takashi Iwai
Cc: Connor McAdams, Takashi Sakamoto, ALSA Development Mailing List,
Alastair Bridgewater, Nathan Chancellor, Nick Desaulniers,
clang-built-linux, Jaroslav Kysela, Linux Kernel Mailing List
On Fri, Mar 22, 2019 at 4:00 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Fri, 22 Mar 2019 15:06:28 +0100,
> Arnd Bergmann wrote:
> >
> > When CONFIG_PCI is disabled, clang gets confused about the
> > control flow of the switch() statement always ending up
> > in the default case, and warns:
> >
> > sound/pci/hda/patch_ca0132.c:7558:6: error: variable 'fw_entry' is used uninitialized whenever 'if' condition is false
> > [-Werror,-Wsometimes-uninitialized]
> > if (!spec->alt_firmware_present) {
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > sound/pci/hda/patch_ca0132.c:7565:42: note: uninitialized use occurs here
> > dsp_os_image = (struct dsp_image_seg *)(fw_entry->data);
> > ^~~~~~~~
> > sound/pci/hda/patch_ca0132.c:7558:2: note: remove the 'if' if its condition is always true
> > if (!spec->alt_firmware_present) {
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > sound/pci/hda/patch_ca0132.c:7521:33: note: initialize the variable 'fw_entry' to silence this warning
> > const struct firmware *fw_entry;
> > ^
> > = NULL
> >
> > Adding an explicit check for CONFIG_PCI avoids the issue.
> > Unfortunately this is not very intuitive here.
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=41197#c1
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > Any suggestions for other workarounds appreciated. If you can think
> > of a better fix, please treat this as a reported-by:
>
> Can it be addressed by the code simplification like below, instead?
>
Yes, I confirmed this works as well, and it's probably more
obvious what's going on, so let's use your version.
Thanks,
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning
2019-03-22 15:00 ` Takashi Iwai
2019-03-22 15:12 ` Arnd Bergmann
@ 2019-03-22 15:13 ` Nathan Chancellor
1 sibling, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2019-03-22 15:13 UTC (permalink / raw)
To: Takashi Iwai
Cc: Arnd Bergmann, Connor McAdams, Takashi Sakamoto, alsa-devel,
Alastair Bridgewater, Nick Desaulniers, clang-built-linux,
Jaroslav Kysela, linux-kernel
On Fri, Mar 22, 2019 at 04:00:19PM +0100, Takashi Iwai wrote:
> On Fri, 22 Mar 2019 15:06:28 +0100,
> Arnd Bergmann wrote:
> >
> > When CONFIG_PCI is disabled, clang gets confused about the
> > control flow of the switch() statement always ending up
> > in the default case, and warns:
> >
> > sound/pci/hda/patch_ca0132.c:7558:6: error: variable 'fw_entry' is used uninitialized whenever 'if' condition is false
> > [-Werror,-Wsometimes-uninitialized]
> > if (!spec->alt_firmware_present) {
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > sound/pci/hda/patch_ca0132.c:7565:42: note: uninitialized use occurs here
> > dsp_os_image = (struct dsp_image_seg *)(fw_entry->data);
> > ^~~~~~~~
> > sound/pci/hda/patch_ca0132.c:7558:2: note: remove the 'if' if its condition is always true
> > if (!spec->alt_firmware_present) {
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > sound/pci/hda/patch_ca0132.c:7521:33: note: initialize the variable 'fw_entry' to silence this warning
> > const struct firmware *fw_entry;
> > ^
> > = NULL
> >
> > Adding an explicit check for CONFIG_PCI avoids the issue.
> > Unfortunately this is not very intuitive here.
> >
> > Link: https://bugs.llvm.org/show_bug.cgi?id=41197#c1
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > Any suggestions for other workarounds appreciated. If you can think
> > of a better fix, please treat this as a reported-by:
>
> Can it be addressed by the code simplification like below, instead?
>
Yes, the warning is fixed with that diff and makes the code easier to
follow I think.
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
>
> thanks,
>
> Takashi
>
> ---
> sound/pci/hda/patch_ca0132.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
> index 29882bda7632..e1ebc6d5f382 100644
> --- a/sound/pci/hda/patch_ca0132.c
> +++ b/sound/pci/hda/patch_ca0132.c
> @@ -1005,7 +1005,6 @@ struct ca0132_spec {
> unsigned int scp_resp_header;
> unsigned int scp_resp_data[4];
> unsigned int scp_resp_count;
> - bool alt_firmware_present;
> bool startup_check_entered;
> bool dsp_reload;
>
> @@ -7518,7 +7517,7 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
> bool dsp_loaded = false;
> struct ca0132_spec *spec = codec->spec;
> const struct dsp_image_seg *dsp_os_image;
> - const struct firmware *fw_entry;
> + const struct firmware *fw_entry = NULL;
> /*
> * Alternate firmwares for different variants. The Recon3Di apparently
> * can use the default firmware, but I'll leave the option in case
> @@ -7529,33 +7528,26 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
> case QUIRK_R3D:
> case QUIRK_AE5:
> if (request_firmware(&fw_entry, DESKTOP_EFX_FILE,
> - codec->card->dev) != 0) {
> + codec->card->dev) != 0)
> codec_dbg(codec, "Desktop firmware not found.");
> - spec->alt_firmware_present = false;
> - } else {
> + else
> codec_dbg(codec, "Desktop firmware selected.");
> - spec->alt_firmware_present = true;
> - }
> break;
> case QUIRK_R3DI:
> if (request_firmware(&fw_entry, R3DI_EFX_FILE,
> - codec->card->dev) != 0) {
> + codec->card->dev) != 0)
> codec_dbg(codec, "Recon3Di alt firmware not detected.");
> - spec->alt_firmware_present = false;
> - } else {
> + else
> codec_dbg(codec, "Recon3Di firmware selected.");
> - spec->alt_firmware_present = true;
> - }
> break;
> default:
> - spec->alt_firmware_present = false;
> break;
> }
> /*
> * Use default ctefx.bin if no alt firmware is detected, or if none
> * exists for your particular codec.
> */
> - if (!spec->alt_firmware_present) {
> + if (!fw_entry) {
> codec_dbg(codec, "Default firmware selected.");
> if (request_firmware(&fw_entry, EFX_FILE,
> codec->card->dev) != 0)
> --
> 2.16.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-03-22 15:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 14:06 [PATCH] ALSA: hda/ca0132: work around clang -Wuninitialized warning Arnd Bergmann
2019-03-22 14:53 ` Nathan Chancellor
2019-03-22 15:00 ` Takashi Iwai
2019-03-22 15:12 ` Arnd Bergmann
2019-03-22 15:13 ` Nathan Chancellor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).