[alsa-devel] [PATCH lib 0/5] alsa-lib topology trivial fixes
Hi,
here is a series of trivial fixes that have been spotted by gcc warnings.
Takashi
Takashi Iwai (5): topology: builder: Fix possibly uninitialized variable in write_elem_block() topology: ctl: Fix access type checks topology: data: Fix wrong size check in tplg_parse_data_hex() topology: parser: Add missing return value to snd_tplg_set_manifest_data() topology: pcm: Remove unused variables
src/topology/builder.c | 9 ++++----- src/topology/ctl.c | 2 +- src/topology/data.c | 4 ++-- src/topology/parser.c | 1 + src/topology/pcm.c | 3 +-- 5 files changed, 9 insertions(+), 10 deletions(-)
When an empty list is passed to write_elem_block(), it may leave vendor_type uninitialized. builder.c: In function ‘write_elem_block’: builder.c:127:8: warning: ‘vendor_type’ may be used uninitialized in this function [-Wmaybe-uninitialized] ret = write_block_header(tplg, tplg_type, vendor_type, ^ builder.c:114:33: note: ‘vendor_type’ was declared here int ret, wsize = 0, count = 0, vendor_type; ^
Add an immediate return for count = 0 for avoiding it, and simplify the code initializing vendor_type without using a one-shot loop.
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/topology/builder.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/topology/builder.c b/src/topology/builder.c index 3bccd44827cc..91412aadd098 100644 --- a/src/topology/builder.c +++ b/src/topology/builder.c @@ -116,13 +116,12 @@ static int write_elem_block(snd_tplg_t *tplg, /* count number of elements */ list_for_each(pos, base) count++; + if (!count) + return 0;
/* write the header for this block */ - list_for_each(pos, base) { - elem = list_entry(pos, struct tplg_elem, list); - vendor_type = elem->vendor_type; - break; - } + elem = list_entry(base->next, struct tplg_elem, list); + vendor_type = elem->vendor_type;
ret = write_block_header(tplg, tplg_type, vendor_type, SND_SOC_TPLG_ABI_VERSION, 0, size, count);
Fix the wrong bit-and check by adding parentheses properly: ctl.c: In function ‘tplg_add_bytes’: ctl.c:868:22: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses] if (be->hdr.access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE ^
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/topology/ctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/topology/ctl.c b/src/topology/ctl.c index 68c4ce5803d1..7d8787f347b8 100644 --- a/src/topology/ctl.c +++ b/src/topology/ctl.c @@ -865,7 +865,7 @@ int tplg_add_bytes(snd_tplg_t *tplg, struct snd_tplg_bytes_template *bytes_ctl,
/* check on TLV bytes control */ if (be->hdr.access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) { - if (be->hdr.access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE + if ((be->hdr.access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) != SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) { SNDERR("error: Invalid TLV bytes control access 0x%x\n", be->hdr.access);
A wrong, uninitialized variable is referred as the size to check in tplg_parse_data_hex(). Spotted by gcc warning: data.c: In function ‘tplg_parse_data_hex’: data.c:228:5: warning: ‘esize’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (esize > TPLG_MAX_PRIV_SIZE) { ^ data.c:211:12: note: ‘esize’ was declared here int size, esize, off, num; ^
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/topology/data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/topology/data.c b/src/topology/data.c index 4ee1f8a15f95..370c0faead36 100644 --- a/src/topology/data.c +++ b/src/topology/data.c @@ -225,8 +225,8 @@ static int tplg_parse_data_hex(snd_config_t *cfg, struct tplg_elem *elem, size = num * width; priv = elem->data;
- if (esize > TPLG_MAX_PRIV_SIZE) { - SNDERR("error: data too big %d\n", esize); + if (size > TPLG_MAX_PRIV_SIZE) { + SNDERR("error: data too big %d\n", size); return -EINVAL; }
Spotted by gcc warning: parser.c: In function ‘snd_tplg_set_manifest_data’: parser.c:361:1: warning: control reaches end of non-void function [-Wreturn-type] } ^
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/topology/parser.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/topology/parser.c b/src/topology/parser.c index ca7de0689cea..44c6146b22f2 100644 --- a/src/topology/parser.c +++ b/src/topology/parser.c @@ -358,6 +358,7 @@ int snd_tplg_set_manifest_data(snd_tplg_t *tplg, const void *data, int len) { tplg->manifest.priv.size = len; tplg->manifest_pdata = data; + return 0; }
void snd_tplg_verbose(snd_tplg_t *tplg, int verbose)
Fix gcc warnings: pcm.c: In function ‘tplg_parse_stream_cfg’: pcm.c:160:6: warning: unused variable ‘ret’ [-Wunused-variable] int ret; ^ pcm.c: In function ‘split_format’: pcm.c:267:13: warning: unused variable ‘ret’ [-Wunused-variable] int i = 0, ret; ^
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/topology/pcm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/topology/pcm.c b/src/topology/pcm.c index 6e42aa18b99b..18d5f0b1b040 100644 --- a/src/topology/pcm.c +++ b/src/topology/pcm.c @@ -157,7 +157,6 @@ static int tplg_parse_stream_cfg(snd_tplg_t *tplg ATTRIBUTE_UNUSED, struct snd_soc_tplg_stream *stream; const char *id, *val; snd_pcm_format_t format; - int ret;
snd_config_get_id(cfg, &id);
@@ -264,7 +263,7 @@ static int split_format(struct snd_soc_tplg_stream_caps *caps, char *str) { char *s = NULL; snd_pcm_format_t format; - int i = 0, ret; + int i = 0;
s = strtok(str, ","); while ((s != NULL) && (i < SND_SOC_TPLG_MAX_FORMATS)) {
On Tue, 2015-09-08 at 22:21 +0200, Takashi Iwai wrote:
Hi,
here is a series of trivial fixes that have been spotted by gcc warnings.
Thanks, Takashi. Dont know why we missed these.
Fwiw, I still see some deprecated API warnings for snd_pcm_hwsync() and an unused variable warning :-
make[3]: Entering directory '/home/lrg/source/alsa-lib/modules/mixer/simple' CC sbase.lo sbase.c: In function 'simple_event_add1': sbase.c:327:16: warning: variable 'values' set but not used [-Wunused-but-set-variable] unsigned long values;
Thanks
Liam
On Wed, 09 Sep 2015 12:08:57 +0200, Liam Girdwood wrote:
On Tue, 2015-09-08 at 22:21 +0200, Takashi Iwai wrote:
Hi,
here is a series of trivial fixes that have been spotted by gcc warnings.
Thanks, Takashi. Dont know why we missed these.
I seem to have forgotten to push to git tree. Now there.
Fwiw, I still see some deprecated API warnings for snd_pcm_hwsync() and an unused variable warning :-
make[3]: Entering directory '/home/lrg/source/alsa-lib/modules/mixer/simple' CC sbase.lo sbase.c: In function 'simple_event_add1': sbase.c:327:16: warning: variable 'values' set but not used [-Wunused-but-set-variable] unsigned long values;
The former is tricky because its the definition of the deprecated function itself. The latter must be a false-positive, as far as I see the code...
Takashi
On Wed, 2015-09-09 at 12:19 +0200, Takashi Iwai wrote:
On Wed, 09 Sep 2015 12:08:57 +0200, Liam Girdwood wrote:
On Tue, 2015-09-08 at 22:21 +0200, Takashi Iwai wrote:
Hi,
here is a series of trivial fixes that have been spotted by gcc warnings.
Thanks, Takashi. Dont know why we missed these.
I seem to have forgotten to push to git tree. Now there.
Oh, I didn't see any warnings before you pushed
make[2]: Entering directory '/home/lrg/source/alsa-lib/src/topology' CC parser.lo CC builder.lo CC ctl.lo CC dapm.lo CC pcm.lo CC data.lo CC text.lo CC channel.lo CC ops.lo CC elem.lo CCLD libtopology.la make[2]: Leaving directory '/home/lrg/source/alsa-lib/src/topology'
Could be our configure settings for some gcc flags are different ?
Liam
On Wed, 09 Sep 2015 12:33:04 +0200, Liam Girdwood wrote:
On Wed, 2015-09-09 at 12:19 +0200, Takashi Iwai wrote:
On Wed, 09 Sep 2015 12:08:57 +0200, Liam Girdwood wrote:
On Tue, 2015-09-08 at 22:21 +0200, Takashi Iwai wrote:
Hi,
here is a series of trivial fixes that have been spotted by gcc warnings.
Thanks, Takashi. Dont know why we missed these.
I seem to have forgotten to push to git tree. Now there.
Oh, I didn't see any warnings before you pushed
make[2]: Entering directory '/home/lrg/source/alsa-lib/src/topology' CC parser.lo CC builder.lo CC ctl.lo CC dapm.lo CC pcm.lo CC data.lo CC text.lo CC channel.lo CC ops.lo CC elem.lo CCLD libtopology.la make[2]: Leaving directory '/home/lrg/source/alsa-lib/src/topology'
Could be our configure settings for some gcc flags are different ?
Maybe because of gcc version. Mine is the new gcc-5.1.1. Each specific -W option is found in each commit log, but -Wall enables all these. Another bug was found with -Wlogical-op, and this seems not enabled with -Wall.
Takashi
participants (2)
-
Liam Girdwood -
Takashi Iwai