alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] Fixed issues/defects reported by Coverity tool
@ 2011-03-17  9:34 sudarshan.bisht
  2011-03-17  9:34 ` [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker sudarshan.bisht
  0 siblings, 1 reply; 4+ messages in thread
From: sudarshan.bisht @ 2011-03-17  9:34 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 got fixes for many potential memory leaks. 


Sudarshan (1):
  alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK"
    checker.

 modules/mixer/simple/sbase.c   |    1 +
 modules/mixer/simple/sbasedl.c |    2 +-
 src/conf.c                     |    2 ++
 src/control/control_hw.c       |    4 +++-
 src/pcm/pcm_file.c             |    5 +++--
 src/pcm/pcm_ladspa.c           |   20 ++++++++++++++------
 src/pcm/pcm_plug.c             |    5 ++++-
 src/pcm/pcm_rate.c             |    1 +
 src/rawmidi/rawmidi.c          |    4 +++-
 9 files changed, 32 insertions(+), 12 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker.
  2011-03-17  9:34 [PATCH 0/1] Fixed issues/defects reported by Coverity tool sudarshan.bisht
@ 2011-03-17  9:34 ` sudarshan.bisht
  2011-03-17 11:53   ` Clemens Ladisch
  0 siblings, 1 reply; 4+ messages in thread
From: sudarshan.bisht @ 2011-03-17  9:34 UTC (permalink / raw)
  To: alsa-devel

From: Sudarshan <sudarshan.bisht@nokia.com>

---
 modules/mixer/simple/sbase.c   |    1 +
 modules/mixer/simple/sbasedl.c |    2 +-
 src/conf.c                     |    2 ++
 src/control/control_hw.c       |    4 +++-
 src/pcm/pcm_file.c             |    5 +++--
 src/pcm/pcm_ladspa.c           |   20 ++++++++++++++------
 src/pcm/pcm_plug.c             |    5 ++++-
 src/pcm/pcm_rate.c             |    1 +
 src/rawmidi/rawmidi.c          |    4 +++-
 9 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/modules/mixer/simple/sbase.c b/modules/mixer/simple/sbase.c
index 97feee8..bb2f59d 100644
--- a/modules/mixer/simple/sbase.c
+++ b/modules/mixer/simple/sbase.c
@@ -377,6 +377,7 @@ static int simple_event_add1(snd_mixer_class_t *class,
 		if (ctype != SND_CTL_ELEM_TYPE_BOOLEAN) {
 		      __invalid_type:
 		      	snd_mixer_selem_id_free(id);
+			free(hsimple);
 			return -EINVAL;
 		}
 		break;
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..61b03ec 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -638,6 +638,7 @@ static int get_char_skip_comments(input_t *input)
 			fd = malloc(sizeof(*fd));
 			if (!fd) {
 				free(str);
+				snd_input_close(in);
 				return -ENOMEM;
 			}
 			fd->name = str;
@@ -4589,6 +4590,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
 		if (err < 0) {
 		_err:
 			free(val);
+			snd_config_delete(sub);
 			return err;
 		}
 		free(val);
diff --git a/src/control/control_hw.c b/src/control/control_hw.c
index cf258b4..281b1a5 100644
--- a/src/control/control_hw.c
+++ b/src/control/control_hw.c
@@ -230,8 +230,10 @@ static int snd_ctl_hw_elem_tlv(snd_ctl_t *handle, int op_flag,
 		return -errno;
 	}
 	if (op_flag == 0) {
-		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size)
+		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) {
+			free(xtlv);
 			return -EFAULT;
+	}
 		memcpy(tlv, xtlv->tlv, xtlv->tlv[1] + 2 * sizeof(unsigned int));
 	}
 	free(xtlv);
diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c
index bfa1cc8..cb61ab9 100644
--- a/src/pcm/pcm_file.c
+++ b/src/pcm/pcm_file.c
@@ -218,7 +218,7 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file)
 
 	if (file->final_fname[0] == '|') {
 		/* pipe mode */
-		FILE *pipe;
+		FILE *pipe = NULL;
 		/* clearing */
 		pipe = popen(file->final_fname + 1, "w");
 		if (!pipe) {
@@ -226,7 +226,8 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file)
 					file->final_fname);
 			return -errno;
 		}
-		fd = fileno(pipe);
+		fd = dup(fileno(pipe));
+		fclose(pipe);
 	} else {
 		if (file->trunc)
 			fd = open(file->final_fname, O_WRONLY|O_CREAT|O_TRUNC,
diff --git a/src/pcm/pcm_ladspa.c b/src/pcm/pcm_ladspa.c
index c413c10..41e1d38 100644
--- a/src/pcm/pcm_ladspa.c
+++ b/src/pcm/pcm_ladspa.c
@@ -750,8 +750,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
                         if (instance->input.data == NULL ||
                             instance->input.m_data == NULL ||
                             instance->output.data == NULL ||
-                            instance->output.m_data == NULL)
+                            instance->output.m_data == NULL) {
+				free(pchannels);
                                 return -ENOMEM;
+			}
 			for (idx = 0; idx < instance->input.channels.size; idx++) {
 			        chn = instance->output.channels.array[idx];
 			        if (pchannels[chn] == NULL && chn < ichannels) {
@@ -761,8 +763,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
 			        instance->input.data[idx] = pchannels[chn];
 			        if (instance->input.data[idx] == NULL) {
                                         instance->input.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 0);
-                                        if (instance->input.data[idx] == NULL)
-                                                return -ENOMEM;
+                                        if (instance->input.data[idx] == NULL) {
+                                                free(pchannels);
+						return -ENOMEM;
+					}
                                 }
                         }
                         for (idx = 0; idx < instance->output.channels.size; idx++) {
@@ -770,8 +774,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
                                 /* FIXME/OPTIMIZE: check if we can remove double alloc */
                                 /* if LADSPA plugin has no broken inplace */
                                 instance->output.data[idx] = malloc(sizeof(LADSPA_Data) * ladspa->allocated);
-                                if (instance->output.data[idx] == NULL)
-                                        return -ENOMEM;
+                                if (instance->output.data[idx] == NULL) {
+                                 	free(pchannels);
+					return -ENOMEM;
+				}
                                 pchannels[chn] = instance->output.m_data[idx] = instance->output.data[idx];
                         }
 		}
@@ -793,8 +799,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
                                                 instance->output.data[idx] = NULL;
                                         } else {
                                                 instance->output.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 1);
-                                                if (instance->output.data[idx] == NULL)
+                                                if (instance->output.data[idx] == NULL) {
+							free(pchannels);	
                                                         return -ENOMEM;
+						}
                                         }
                                 }
                         }
diff --git a/src/pcm/pcm_plug.c b/src/pcm/pcm_plug.c
index e9d2923..7308962 100644
--- a/src/pcm/pcm_plug.c
+++ b/src/pcm/pcm_plug.c
@@ -1298,6 +1298,7 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
 		err = snd_pcm_route_load_ttable(tt, ttable, csize, ssize, &cused, &sused, -1);
 		if (err < 0) {
 			snd_config_delete(sconf);
+			free(ttable);
 			return err;
 		}
 	}
@@ -1310,8 +1311,10 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
 
 	err = snd_pcm_open_slave(&spcm, root, sconf, stream, mode, conf);
 	snd_config_delete(sconf);
-	if (err < 0)
+	if (err < 0) {
+		free(ttable);
 		return err;
+	}
 	err = snd_pcm_plug_open(pcmp, name, sformat, schannels, srate, rate_converter,
 				route_policy, ttable, ssize, cused, sused, spcm, 1);
 	if (err < 0)
diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index 70e30e5..065c9da 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -1392,6 +1392,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name,
 	} else {
 		SNDERR("Invalid type for rate converter");
 		snd_pcm_close(pcm);
+		free(rate);
 		return -EINVAL;
 	}
 	if (err < 0) {
diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c
index b28488a..25732d2 100644
--- a/src/rawmidi/rawmidi.c
+++ b/src/rawmidi/rawmidi.c
@@ -255,8 +255,10 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp
 		snd_config_delete(type_conf);
 	if (err >= 0)
 		err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
-	if (err < 0)
+	if (err < 0) {
+		snd_dlclose(h);
 		return err;
+	}
 	if (inputp) {
 		(*inputp)->dl_handle = h; h = NULL;
 		snd_rawmidi_params_default(*inputp, &params);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker.
  2011-03-17  9:34 ` [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker sudarshan.bisht
@ 2011-03-17 11:53   ` Clemens Ladisch
  2011-03-18  8:23     ` Sudarshan Bisht
  0 siblings, 1 reply; 4+ messages in thread
From: Clemens Ladisch @ 2011-03-17 11:53 UTC (permalink / raw)
  To: sudarshan.bisht; +Cc: alsa-devel

sudarshan.bisht@nokia.com wrote:
> --- a/modules/mixer/simple/sbase.c
> +++ b/modules/mixer/simple/sbase.c
> @@ -377,6 +377,7 @@ static int simple_event_add1(snd_mixer_class_t *class,
>  		if (ctype != SND_CTL_ELEM_TYPE_BOOLEAN) {
>  		      __invalid_type:
>  		      	snd_mixer_selem_id_free(id);
> +			free(hsimple);
>  			return -EINVAL;
>  		}
>  		break;

OK

> --- 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;

OK

> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -638,6 +638,7 @@ static int get_char_skip_comments(input_t *input)
>  			fd = malloc(sizeof(*fd));
>  			if (!fd) {
>  				free(str);
> +				snd_input_close(in);
>  				return -ENOMEM;
>  			}

OK

> @@ -4589,6 +4590,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
>  		if (err < 0) {
>  		_err:
>  			free(val);
> +			snd_config_delete(sub);
>  			return err;
>  		}

When this code is reached through the label _err, sub is not
necessarily a valid pointer.

> --- a/src/control/control_hw.c
> +++ b/src/control/control_hw.c
> @@ -230,8 +230,10 @@ static int snd_ctl_hw_elem_tlv(snd_ctl_t *handle, int op_flag,
>  		return -errno;
>  	}
>  	if (op_flag == 0) {
> -		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size)
> +		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) {
> +			free(xtlv);
>  			return -EFAULT;
> +	}

OK, but the indent of the closing brace is wrong.

> --- a/src/pcm/pcm_file.c
> +++ b/src/pcm/pcm_file.c
> @@ -218,7 +218,7 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file)
>  
>  	if (file->final_fname[0] == '|') {
>  		/* pipe mode */
> -		FILE *pipe;
> +		FILE *pipe = NULL;
>  		/* clearing */
>  		pipe = popen(file->final_fname + 1, "w");
>  		if (!pipe) {

This isn't necessary because the variable is set in the next line.

> @@ -226,7 +226,8 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file)
>  					file->final_fname);
>  			return -errno;
>  		}
> -		fd = fileno(pipe);
> +		fd = dup(fileno(pipe));
> +		fclose(pipe);
>  	} else {
>  		if (file->trunc)
>  			fd = open(file->final_fname, O_WRONLY|O_CREAT|O_TRUNC,

OK, but dup() might fail.

> --- a/src/pcm/pcm_ladspa.c
> +++ b/src/pcm/pcm_ladspa.c
> @@ -750,8 +750,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
>                          if (instance->input.data == NULL ||
>                              instance->input.m_data == NULL ||
>                              instance->output.data == NULL ||
> -                            instance->output.m_data == NULL)
> +                            instance->output.m_data == NULL) {
> +				free(pchannels);
>                                  return -ENOMEM;
> +			}

OK

> @@ -761,8 +763,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
>  			        instance->input.data[idx] = pchannels[chn];
>  			        if (instance->input.data[idx] == NULL) {
>                                          instance->input.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 0);
> -                                        if (instance->input.data[idx] == NULL)
> -                                                return -ENOMEM;
> +                                        if (instance->input.data[idx] == NULL) {
> +                                                free(pchannels);
> +						return -ENOMEM;
> +					}
>                                  }
>                          }

OK

> @@ -770,8 +774,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
>                                  /* FIXME/OPTIMIZE: check if we can remove double alloc */
>                                  /* if LADSPA plugin has no broken inplace */
>                                  instance->output.data[idx] = malloc(sizeof(LADSPA_Data) * ladspa->allocated);
> -                                if (instance->output.data[idx] == NULL)
> -                                        return -ENOMEM;
> +                                if (instance->output.data[idx] == NULL) {
> +                                 	free(pchannels);
> +					return -ENOMEM;
> +				}
>                                  pchannels[chn] = instance->output.m_data[idx] = instance->output.data[idx];
>                          }
>  		}

OK

> @@ -793,8 +799,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
>                                                  instance->output.data[idx] = NULL;
>                                          } else {
>                                                  instance->output.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 1);
> -                                                if (instance->output.data[idx] == NULL)
> +                                                if (instance->output.data[idx] == NULL) {
> +							free(pchannels);	
>                                                          return -ENOMEM;
> +						}
>                                          }
>                                  }
>                          }

OK

> --- a/src/pcm/pcm_plug.c
> +++ b/src/pcm/pcm_plug.c
> @@ -1298,6 +1298,7 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
>  		err = snd_pcm_route_load_ttable(tt, ttable, csize, ssize, &cused, &sused, -1);
>  		if (err < 0) {
>  			snd_config_delete(sconf);
> +			free(ttable);
>  			return err;
>  		}

OK


> @@ -1310,8 +1311,10 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
>  
>  	err = snd_pcm_open_slave(&spcm, root, sconf, stream, mode, conf);
>  	snd_config_delete(sconf);
> -	if (err < 0)
> +	if (err < 0) {
> +		free(ttable);
>  		return err;
> +	}

OK, but the following snd_pcm_plug_open() does not guarantee that ttable
is freed if an error happens.

> diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
> index 70e30e5..065c9da 100644
> --- a/src/pcm/pcm_rate.c
> +++ b/src/pcm/pcm_rate.c
> @@ -1392,6 +1392,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name,
>  	} else {
>  		SNDERR("Invalid type for rate converter");
>  		snd_pcm_close(pcm);
> +		free(rate);
>  		return -EINVAL;
>  	}

OK, but the following three error cases don't free rate either.

> --- a/src/rawmidi/rawmidi.c
> +++ b/src/rawmidi/rawmidi.c
> @@ -255,8 +255,10 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp
>  		snd_config_delete(type_conf);
>  	if (err >= 0)
>  		err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
> -	if (err < 0)
> +	if (err < 0) {
> +		snd_dlclose(h);
>  		return err;
> +	}
>  	if (inputp) {
>  		(*inputp)->dl_handle = h; h = NULL;
>  		snd_rawmidi_params_default(*inputp, &params);

OK


Regards,
Clemens

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker.
  2011-03-17 11:53   ` Clemens Ladisch
@ 2011-03-18  8:23     ` Sudarshan Bisht
  0 siblings, 0 replies; 4+ messages in thread
From: Sudarshan Bisht @ 2011-03-18  8:23 UTC (permalink / raw)
  To: ext Clemens Ladisch; +Cc: alsa-devel

On Thu, 2011-03-17 at 12:53 +0100, ext Clemens Ladisch wrote:
> sudarshan.bisht@nokia.com wrote:
> > --- a/modules/mixer/simple/sbase.c
> > +++ b/modules/mixer/simple/sbase.c
> > @@ -377,6 +377,7 @@ static int simple_event_add1(snd_mixer_class_t *class,
> >  		if (ctype != SND_CTL_ELEM_TYPE_BOOLEAN) {
> >  		      __invalid_type:
> >  		      	snd_mixer_selem_id_free(id);
> > +			free(hsimple);
> >  			return -EINVAL;
> >  		}
> >  		break;
> 
> OK
> 
> > --- 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;
> 
> OK
> 
> > --- a/src/conf.c
> > +++ b/src/conf.c
> > @@ -638,6 +638,7 @@ static int get_char_skip_comments(input_t *input)
> >  			fd = malloc(sizeof(*fd));
> >  			if (!fd) {
> >  				free(str);
> > +				snd_input_close(in);
> >  				return -ENOMEM;
> >  			}
> 
> OK
> 
> > @@ -4589,6 +4590,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
> >  		if (err < 0) {
> >  		_err:
> >  			free(val);
> > +			snd_config_delete(sub);
> >  			return err;
> >  		}
> 
> When this code is reached through the label _err, sub is not
> necessarily a valid pointer.
Ok, then a check for the validity of sub can be put here. 
> 
> > --- a/src/control/control_hw.c
> > +++ b/src/control/control_hw.c
> > @@ -230,8 +230,10 @@ static int snd_ctl_hw_elem_tlv(snd_ctl_t *handle, int op_flag,
> >  		return -errno;
> >  	}
> >  	if (op_flag == 0) {
> > -		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size)
> > +		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) {
> > +			free(xtlv);
> >  			return -EFAULT;
> > +	}
> 
> OK, but the indent of the closing brace is wrong.
Ok, will correct it. 
> 
> > --- a/src/pcm/pcm_file.c
> > +++ b/src/pcm/pcm_file.c
> > @@ -218,7 +218,7 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file)
> >  
> >  	if (file->final_fname[0] == '|') {
> >  		/* pipe mode */
> > -		FILE *pipe;
> > +		FILE *pipe = NULL;
> >  		/* clearing */
> >  		pipe = popen(file->final_fname + 1, "w");
> >  		if (!pipe) {
> 
> This isn't necessary because the variable is set in the next line.
Ok.
> 
> > @@ -226,7 +226,8 @@ static int snd_pcm_file_open_output_file(snd_pcm_file_t *file)
> >  					file->final_fname);
> >  			return -errno;
> >  		}
> > -		fd = fileno(pipe);
> > +		fd = dup(fileno(pipe));
> > +		fclose(pipe);
> >  	} else {
> >  		if (file->trunc)
> >  			fd = open(file->final_fname, O_WRONLY|O_CREAT|O_TRUNC,
> 
> OK, but dup() might fail.
Yes, will take care of that. 
> 
> > --- a/src/pcm/pcm_ladspa.c
> > +++ b/src/pcm/pcm_ladspa.c
> > @@ -750,8 +750,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
> >                          if (instance->input.data == NULL ||
> >                              instance->input.m_data == NULL ||
> >                              instance->output.data == NULL ||
> > -                            instance->output.m_data == NULL)
> > +                            instance->output.m_data == NULL) {
> > +				free(pchannels);
> >                                  return -ENOMEM;
> > +			}
> 
> OK
> 
> > @@ -761,8 +763,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
> >  			        instance->input.data[idx] = pchannels[chn];
> >  			        if (instance->input.data[idx] == NULL) {
> >                                          instance->input.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 0);
> > -                                        if (instance->input.data[idx] == NULL)
> > -                                                return -ENOMEM;
> > +                                        if (instance->input.data[idx] == NULL) {
> > +                                                free(pchannels);
> > +						return -ENOMEM;
> > +					}
> >                                  }
> >                          }
> 
> OK
> 
> > @@ -770,8 +774,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
> >                                  /* FIXME/OPTIMIZE: check if we can remove double alloc */
> >                                  /* if LADSPA plugin has no broken inplace */
> >                                  instance->output.data[idx] = malloc(sizeof(LADSPA_Data) * ladspa->allocated);
> > -                                if (instance->output.data[idx] == NULL)
> > -                                        return -ENOMEM;
> > +                                if (instance->output.data[idx] == NULL) {
> > +                                 	free(pchannels);
> > +					return -ENOMEM;
> > +				}
> >                                  pchannels[chn] = instance->output.m_data[idx] = instance->output.data[idx];
> >                          }
> >  		}
> 
> OK
> 
> > @@ -793,8 +799,10 @@ static int snd_pcm_ladspa_allocate_memory(snd_pcm_t *pcm, snd_pcm_ladspa_t *lads
> >                                                  instance->output.data[idx] = NULL;
> >                                          } else {
> >                                                  instance->output.data[idx] = snd_pcm_ladspa_allocate_zero(ladspa, 1);
> > -                                                if (instance->output.data[idx] == NULL)
> > +                                                if (instance->output.data[idx] == NULL) {
> > +							free(pchannels);	
> >                                                          return -ENOMEM;
> > +						}
> >                                          }
> >                                  }
> >                          }
> 
> OK
> 
> > --- a/src/pcm/pcm_plug.c
> > +++ b/src/pcm/pcm_plug.c
> > @@ -1298,6 +1298,7 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
> >  		err = snd_pcm_route_load_ttable(tt, ttable, csize, ssize, &cused, &sused, -1);
> >  		if (err < 0) {
> >  			snd_config_delete(sconf);
> > +			free(ttable);
> >  			return err;
> >  		}
> 
> OK
> 
> 
> > @@ -1310,8 +1311,10 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name,
> >  
> >  	err = snd_pcm_open_slave(&spcm, root, sconf, stream, mode, conf);
> >  	snd_config_delete(sconf);
> > -	if (err < 0)
> > +	if (err < 0) {
> > +		free(ttable);
> >  		return err;
> > +	}
> 
> OK, but the following snd_pcm_plug_open() does not guarantee that ttable
> is freed if an error happens.
Ok. will take care of that too. 
> 
> > diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
> > index 70e30e5..065c9da 100644
> > --- a/src/pcm/pcm_rate.c
> > +++ b/src/pcm/pcm_rate.c
> > @@ -1392,6 +1392,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name,
> >  	} else {
> >  		SNDERR("Invalid type for rate converter");
> >  		snd_pcm_close(pcm);
> > +		free(rate);
> >  		return -EINVAL;
> >  	}
> 
> OK, but the following three error cases don't free rate either.
Yes, these should be freed. 
> 
> > --- a/src/rawmidi/rawmidi.c
> > +++ b/src/rawmidi/rawmidi.c
> > @@ -255,8 +255,10 @@ static int snd_rawmidi_open_conf(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp
> >  		snd_config_delete(type_conf);
> >  	if (err >= 0)
> >  		err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
> > -	if (err < 0)
> > +	if (err < 0) {
> > +		snd_dlclose(h);
> >  		return err;
> > +	}
> >  	if (inputp) {
> >  		(*inputp)->dl_handle = h; h = NULL;
> >  		snd_rawmidi_params_default(*inputp, &params);
> 
> OK
> 
> 
> Regards,
> Clemens

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-03-18  8:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-17  9:34 [PATCH 0/1] Fixed issues/defects reported by Coverity tool sudarshan.bisht
2011-03-17  9:34 ` [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker sudarshan.bisht
2011-03-17 11:53   ` Clemens Ladisch
2011-03-18  8:23     ` 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).