* [PATCH 0/1] Fixed issues/defects reported by Coverity tool.
@ 2011-03-17 9:41 sudarshan.bisht
2011-03-17 9:41 ` [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker sudarshan.bisht
0 siblings, 1 reply; 6+ messages in thread
From: sudarshan.bisht @ 2011-03-17 9:41 UTC (permalink / raw)
To: alsa-devel
From: Sudarshan <sudarshan.bisht@nokia.com>
Coverity Static Analysis helps developers find hard-to-spot,
yet potentially crash-causing defects early in the development phase,
reducing the cost,time, and risk of software errors.
This patch has fix for situations where variable can be NULL
but not been checked beforehand.
Sudarshan (1):
alsa-lib: fixed coverity reported issues under "FORWARD_NULL"
checker.
modules/mixer/simple/sbasedl.c | 2 +-
src/conf.c | 8 +++++---
src/hwdep/hwdep.c | 2 +-
src/pcm/pcm_hooks.c | 5 +++--
src/pcm/pcm_simple.c | 2 +-
src/rawmidi/rawmidi.c | 2 +-
src/rawmidi/rawmidi_virt.c | 8 +++++---
7 files changed, 17 insertions(+), 12 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker. 2011-03-17 9:41 [PATCH 0/1] Fixed issues/defects reported by Coverity tool sudarshan.bisht @ 2011-03-17 9:41 ` sudarshan.bisht 2011-03-17 12:08 ` Clemens Ladisch 0 siblings, 1 reply; 6+ messages in thread From: sudarshan.bisht @ 2011-03-17 9:41 UTC (permalink / raw) To: alsa-devel From: Sudarshan <sudarshan.bisht@nokia.com> --- modules/mixer/simple/sbasedl.c | 2 +- src/conf.c | 8 +++++--- src/hwdep/hwdep.c | 2 +- src/pcm/pcm_hooks.c | 5 +++-- src/pcm/pcm_simple.c | 2 +- src/rawmidi/rawmidi.c | 2 +- src/rawmidi/rawmidi_virt.c | 8 +++++--- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/modules/mixer/simple/sbasedl.c b/modules/mixer/simple/sbasedl.c index 0137586..494802f 100644 --- a/modules/mixer/simple/sbasedl.c +++ b/modules/mixer/simple/sbasedl.c @@ -99,7 +99,7 @@ int mixer_simple_basic_dlopen(snd_mixer_class_t *class, __error: if (initflag) free(priv); - if (h == NULL) + if (h) snd_dlclose(h); free(xlib); return -ENXIO; diff --git a/src/conf.c b/src/conf.c index 8939d62..9b69e12 100644 --- a/src/conf.c +++ b/src/conf.c @@ -3321,9 +3321,11 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c snd_config_delete(func_conf); if (err >= 0) { snd_config_t *nroot; - err = func(root, config, &nroot, private_data); - if (err < 0) - SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); + if (func) { + err = func(root, config, &nroot, private_data); + if (err < 0) + SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); + } snd_dlclose(h); if (err >= 0 && nroot) err = snd_config_substitute(root, nroot); diff --git a/src/hwdep/hwdep.c b/src/hwdep/hwdep.c index b882b35..690d9f6 100644 --- a/src/hwdep/hwdep.c +++ b/src/hwdep/hwdep.c @@ -130,7 +130,7 @@ static int snd_hwdep_open_conf(snd_hwdep_t **hwdep, _err: if (type_conf) snd_config_delete(type_conf); - if (err >= 0) { + if (err >= 0 && open_func) { err = open_func(hwdep, name, hwdep_root, hwdep_conf, mode); if (err >= 0) { (*hwdep)->dl_handle = h; diff --git a/src/pcm/pcm_hooks.c b/src/pcm/pcm_hooks.c index 3a99d55..a0ed0b9 100644 --- a/src/pcm/pcm_hooks.c +++ b/src/pcm/pcm_hooks.c @@ -445,14 +445,15 @@ static int snd_pcm_hook_add_conf(snd_pcm_t *pcm, snd_config_t *root, snd_config_ else err = install_func(pcm, args); snd_config_delete(args); - } else + } else if (install_func) err = install_func(pcm, args); if (err >= 0) err = hook_add_dlobj(pcm, h); if (err < 0) { - snd_dlclose(h); + if (h) + snd_dlclose(h); return err; } return 0; diff --git a/src/pcm/pcm_simple.c b/src/pcm/pcm_simple.c index 975f699..f943ec0 100644 --- a/src/pcm/pcm_simple.c +++ b/src/pcm/pcm_simple.c @@ -89,7 +89,7 @@ static int set_hw_params(snd_pcm_t *pcm, return err; if (periods == 1) return -EINVAL; - if (*period_time == 0) { + if (period_time) { err = INTERNAL(snd_pcm_hw_params_get_period_time)(hw_params, period_time, NULL); if (err < 0) return err; diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c index b28488a..a8d7231 100644 --- a/src/rawmidi/rawmidi.c +++ b/src/rawmidi/rawmidi.c @@ -253,7 +253,7 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp _err: if (type_conf) snd_config_delete(type_conf); - if (err >= 0) + if (err >= 0 && open_func) err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode); if (err < 0) return err; diff --git a/src/rawmidi/rawmidi_virt.c b/src/rawmidi/rawmidi_virt.c index 52b8984..e5b17e4 100644 --- a/src/rawmidi/rawmidi_virt.c +++ b/src/rawmidi/rawmidi_virt.c @@ -383,9 +383,11 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp, _err: if (seq_handle) snd_seq_close(seq_handle); - if (virt->midi_event) - snd_midi_event_free(virt->midi_event); - free(virt); + if (virt) { + if (virt->midi_event) + snd_midi_event_free(virt->midi_event); + free(virt); + } if (inputp) free(*inputp); if (outputp) -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker. 2011-03-17 9:41 ` [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker sudarshan.bisht @ 2011-03-17 12:08 ` Clemens Ladisch 2011-03-18 9:14 ` Sudarshan Bisht 0 siblings, 1 reply; 6+ messages in thread From: Clemens Ladisch @ 2011-03-17 12:08 UTC (permalink / raw) To: sudarshan.bisht; +Cc: alsa-devel sudarshan.bisht@nokia.com wrote: > --- a/modules/mixer/simple/sbasedl.c > +++ b/modules/mixer/simple/sbasedl.c > @@ -99,7 +99,7 @@ int mixer_simple_basic_dlopen(snd_mixer_class_t *class, > __error: > if (initflag) > free(priv); > - if (h == NULL) > + if (h) > snd_dlclose(h); > free(xlib); > return -ENXIO; We already had this one. > --- a/src/conf.c > +++ b/src/conf.c > @@ -3321,9 +3321,11 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c > snd_config_delete(func_conf); > if (err >= 0) { > snd_config_t *nroot; > - err = func(root, config, &nroot, private_data); > - if (err < 0) > - SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > + if (func) { > + err = func(root, config, &nroot, private_data); > + if (err < 0) > + SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > + } The preceding "!func" and "err >= 0" checks already guarantee that func is valid. > --- a/src/hwdep/hwdep.c > +++ b/src/hwdep/hwdep.c > @@ -130,7 +130,7 @@ static int snd_hwdep_open_conf(snd_hwdep_t **hwdep, > _err: > if (type_conf) > snd_config_delete(type_conf); > - if (err >= 0) { > + if (err >= 0 && open_func) { > err = open_func(hwdep, name, hwdep_root, hwdep_conf, mode); > if (err >= 0) { > (*hwdep)->dl_handle = h; Same here. > --- a/src/pcm/pcm_hooks.c > +++ b/src/pcm/pcm_hooks.c > @@ -445,14 +445,15 @@ static int snd_pcm_hook_add_conf(snd_pcm_t *pcm, snd_config_t *root, snd_config_ > else > err = install_func(pcm, args); > snd_config_delete(args); > - } else > + } else if (install_func) > err = install_func(pcm, args); > > if (err >= 0) > err = hook_add_dlobj(pcm, h); > > if (err < 0) { > - snd_dlclose(h); > + if (h) > + snd_dlclose(h); > return err; > } > return 0; Same here. > --- a/src/pcm/pcm_simple.c > +++ b/src/pcm/pcm_simple.c > @@ -89,7 +89,7 @@ static int set_hw_params(snd_pcm_t *pcm, > return err; > if (periods == 1) > return -EINVAL; > - if (*period_time == 0) { > + if (period_time) { > err = INTERNAL(snd_pcm_hw_params_get_period_time)(hw_params, period_time, NULL); > if (err < 0) > return err; OK, I think, but this change deserves an explanation. > --- a/src/rawmidi/rawmidi.c > +++ b/src/rawmidi/rawmidi.c > @@ -253,7 +253,7 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp > _err: > if (type_conf) > snd_config_delete(type_conf); > - if (err >= 0) > + if (err >= 0 && open_func) > err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode); > if (err < 0) > return err; Same as with the function pointers above. > --- a/src/rawmidi/rawmidi_virt.c > +++ b/src/rawmidi/rawmidi_virt.c > @@ -383,9 +383,11 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp, > _err: > if (seq_handle) > snd_seq_close(seq_handle); > - if (virt->midi_event) > - snd_midi_event_free(virt->midi_event); > - free(virt); > + if (virt) { > + if (virt->midi_event) > + snd_midi_event_free(virt->midi_event); > + free(virt); > + } > if (inputp) > free(*inputp); > if (outputp) OK, but not needed for the free(). Regards, Clemens ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker. 2011-03-17 12:08 ` Clemens Ladisch @ 2011-03-18 9:14 ` Sudarshan Bisht 2011-03-18 14:05 ` Clemens Ladisch 0 siblings, 1 reply; 6+ messages in thread From: Sudarshan Bisht @ 2011-03-18 9:14 UTC (permalink / raw) To: ext Clemens Ladisch; +Cc: alsa-devel On Thu, 2011-03-17 at 13:08 +0100, ext Clemens Ladisch wrote: > sudarshan.bisht@nokia.com wrote: > > --- a/modules/mixer/simple/sbasedl.c > > +++ b/modules/mixer/simple/sbasedl.c > > @@ -99,7 +99,7 @@ int mixer_simple_basic_dlopen(snd_mixer_class_t *class, > > __error: > > if (initflag) > > free(priv); > > - if (h == NULL) > > + if (h) > > snd_dlclose(h); > > free(xlib); > > return -ENXIO; > > We already had this one. Ok. > > > --- a/src/conf.c > > +++ b/src/conf.c > > @@ -3321,9 +3321,11 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c > > snd_config_delete(func_conf); > > if (err >= 0) { > > snd_config_t *nroot; > > - err = func(root, config, &nroot, private_data); > > - if (err < 0) > > - SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > > + if (func) { > > + err = func(root, config, &nroot, private_data); > > + if (err < 0) > > + SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > > + } > > The preceding "!func" and "err >= 0" checks already guarantee that > func is valid. But in case of "goto _err" , "!func" and "err >= 0" are not going to be checked. > > > --- a/src/hwdep/hwdep.c > > +++ b/src/hwdep/hwdep.c > > @@ -130,7 +130,7 @@ static int snd_hwdep_open_conf(snd_hwdep_t **hwdep, > > _err: > > if (type_conf) > > snd_config_delete(type_conf); > > - if (err >= 0) { > > + if (err >= 0 && open_func) { > > err = open_func(hwdep, name, hwdep_root, hwdep_conf, mode); > > if (err >= 0) { > > (*hwdep)->dl_handle = h; > > Same here. Same explanation as above. > > > --- a/src/pcm/pcm_hooks.c > > +++ b/src/pcm/pcm_hooks.c > > @@ -445,14 +445,15 @@ static int snd_pcm_hook_add_conf(snd_pcm_t *pcm, snd_config_t *root, snd_config_ > > else > > err = install_func(pcm, args); > > snd_config_delete(args); > > - } else > > + } else if (install_func) > > err = install_func(pcm, args); > > > > if (err >= 0) > > err = hook_add_dlobj(pcm, h); > > > > if (err < 0) { > > - snd_dlclose(h); > > + if (h) > > + snd_dlclose(h); > > return err; > > } > > return 0; > > Same here. h will be NULL when encounters any preceding "goto _err". > > > --- a/src/pcm/pcm_simple.c > > +++ b/src/pcm/pcm_simple.c > > @@ -89,7 +89,7 @@ static int set_hw_params(snd_pcm_t *pcm, > > return err; > > if (periods == 1) > > return -EINVAL; > > - if (*period_time == 0) { > > + if (period_time) { > > err = INTERNAL(snd_pcm_hw_params_get_period_time)(hw_params, period_time, NULL); > > if (err < 0) > > return err; > > OK, I think, but this change deserves an explanation. If "period_time == NULL" ( in preceding if condition) is true then "if (*period_time == 0)" has no meaning, and if "*period_time == 0" ( in preceding if condition) is true then obviously period_time will be a valid pointer and satisfy the condition. Thats what original source codes demands. > > > --- a/src/rawmidi/rawmidi.c > > +++ b/src/rawmidi/rawmidi.c > > @@ -253,7 +253,7 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp > > _err: > > if (type_conf) > > snd_config_delete(type_conf); > > - if (err >= 0) > > + if (err >= 0 && open_func) > > err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode); > > if (err < 0) > > return err; > > Same as with the function pointers above. Same reason what I gave for function pointers. > > > --- a/src/rawmidi/rawmidi_virt.c > > +++ b/src/rawmidi/rawmidi_virt.c > > @@ -383,9 +383,11 @@ int snd_rawmidi_virtual_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp, > > _err: > > if (seq_handle) > > snd_seq_close(seq_handle); > > - if (virt->midi_event) > > - snd_midi_event_free(virt->midi_event); > > - free(virt); > > + if (virt) { > > + if (virt->midi_event) > > + snd_midi_event_free(virt->midi_event); > > + free(virt); > > + } > > if (inputp) > > free(*inputp); > > if (outputp) > > OK, but not needed for the free(). Sorry, but I did not get this. > > > Regards, > Clemens Br, Sudarshan Bisht. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker. 2011-03-18 9:14 ` Sudarshan Bisht @ 2011-03-18 14:05 ` Clemens Ladisch 2011-03-21 8:29 ` Sudarshan Bisht 0 siblings, 1 reply; 6+ messages in thread From: Clemens Ladisch @ 2011-03-18 14:05 UTC (permalink / raw) To: sudarshan.bisht; +Cc: alsa-devel Sudarshan Bisht wrote: > On Thu, 2011-03-17 at 13:08 +0100, ext Clemens Ladisch wrote: > > sudarshan.bisht@nokia.com wrote: > > > --- a/src/conf.c > > > +++ b/src/conf.c > > > @@ -3321,9 +3321,11 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c > > > snd_config_delete(func_conf); > > > if (err >= 0) { > > > snd_config_t *nroot; > > > - err = func(root, config, &nroot, private_data); > > > - if (err < 0) > > > - SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > > > + if (func) { > > > + err = func(root, config, &nroot, private_data); > > > + if (err < 0) > > > + SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > > > + } > > > > The preceding "!func" and "err >= 0" checks already guarantee that > > func is valid. > > But in case of "goto _err" , "!func" and "err >= 0" are not going to be > checked. "err >= 0" is checked, and in all error cases, err must be negative; if not, this is bug at that place and should be fixed before that goto. I now see that the TYPE_COMPOUND check forgets to set "err = -EINVAL;". Regards, Clemens ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker. 2011-03-18 14:05 ` Clemens Ladisch @ 2011-03-21 8:29 ` Sudarshan Bisht 0 siblings, 0 replies; 6+ messages in thread From: Sudarshan Bisht @ 2011-03-21 8:29 UTC (permalink / raw) To: ext Clemens Ladisch; +Cc: alsa-devel On Fri, 2011-03-18 at 15:05 +0100, ext Clemens Ladisch wrote: > Sudarshan Bisht wrote: > > On Thu, 2011-03-17 at 13:08 +0100, ext Clemens Ladisch wrote: > > > sudarshan.bisht@nokia.com wrote: > > > > --- a/src/conf.c > > > > +++ b/src/conf.c > > > > @@ -3321,9 +3321,11 @@ static int snd_config_hooks_call(snd_config_t *root, snd_config_t *config, snd_c > > > > snd_config_delete(func_conf); > > > > if (err >= 0) { > > > > snd_config_t *nroot; > > > > - err = func(root, config, &nroot, private_data); > > > > - if (err < 0) > > > > - SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > > > > + if (func) { > > > > + err = func(root, config, &nroot, private_data); > > > > + if (err < 0) > > > > + SNDERR("function %s returned error: %s", func_name, snd_strerror(err)); > > > > + } > > > > > > The preceding "!func" and "err >= 0" checks already guarantee that > > > func is valid. > > > > But in case of "goto _err" , "!func" and "err >= 0" are not going to be > > checked. > > "err >= 0" is checked, and in all error cases, err must be negative; > if not, this is bug at that place and should be fixed before that goto. > I now see that the TYPE_COMPOUND check forgets to set "err = -EINVAL;". > Yes, now I got your point, and I will take care of this in all cases. > > Regards, > Clemens ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-21 8:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-17 9:41 [PATCH 0/1] Fixed issues/defects reported by Coverity tool sudarshan.bisht 2011-03-17 9:41 ` [PATCH 1/1] alsa-lib: fixed coverity reported issues under "FORWARD_NULL" checker sudarshan.bisht 2011-03-17 12:08 ` Clemens Ladisch 2011-03-18 9:14 ` Sudarshan Bisht 2011-03-18 14:05 ` Clemens Ladisch 2011-03-21 8:29 ` Sudarshan Bisht
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).