Re: [PATCH 1/1] alsa-lib: fixed coverity reported issues under "RESOURCE_LEAK" checker.

[Clemens Ladisch <clemens@ladisch.de>]

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