coverity fix in alsa-libs

[Renu Tyagi]

Hi,

I ran Coverity analysis tool on alsa and found some bugs.
Bug and Patch description

1. Changed file : aserver.c
Socket not closed before returning when bind fails
Community Code:

if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
return result;
}
return sock;
}

Recommended Code :

if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
close(sock);
return result;
}
return sock;
}

2.Changed file : control_shm.c
Socket not closed before returning when connect fails

Community Code:
if (connect(sock, (struct sockaddr *) addr, size) < 0)
return -errno;
return sock;
}

Recommended Code :
if (connect(sock, (struct sockaddr *) addr, size) < 0){
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}

3.Changed file : pcm_shm.c
Socket not closed before returning when connect fails

Community Code:
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
return -errno;
}
return sock;
}
Recommended Code :
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}

PFA patch.





Thanks & Regards,

Renu Tyagi

[Alexander E. Patrakov]

Post by Renu Tyagi
Hi,
I ran Coverity analysis tool on alsa and found some bugs.

May I suggest that we remove aserver and the shm plugin instead of
applying the patch? Three days ago I tried to use it for testing my fix
to the share plugin, but failed. In other words: if even speaker-test
cannot be made to work on it without crashing and/or hanging or valgrind
errors, then I'd rather be aggressive here.

And next time please CC: Takashi Iwai on all alsa-lib patches :)

Post by Renu Tyagi
Bug and Patch description
1. Changed file : aserver.c
Socket not closed before returning when bind fails
if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
return result;
}
return sock;
}
if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
close(sock);
return result;
}
return sock;
}
2.Changed file : control_shm.c
Socket not closed before returning when connect fails
if (connect(sock, (struct sockaddr *) addr, size) < 0)
return -errno;
return sock;
}
if (connect(sock, (struct sockaddr *) addr, size) < 0){
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}
3.Changed file : pcm_shm.c
Socket not closed before returning when connect fails
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
return -errno;
}
return sock;
}
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}
PFA patch.
Thanks & Regards,
Renu Tyagi
_______________________________________________
Alsa-devel mailing list
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Takashi Iwai]

At Mon, 15 Sep 2014 17:36:02 +0600,

Post by Alexander E. Patrakov

Post by Renu Tyagi
Hi,
I ran Coverity analysis tool on alsa and found some bugs.

May I suggest that we remove aserver and the shm plugin instead of
applying the patch? Three days ago I tried to use it for testing my fix
to the share plugin, but failed. In other words: if even speaker-test
cannot be made to work on it without crashing and/or hanging or valgrind
errors, then I'd rather be aggressive here.
And next time please CC: Takashi Iwai on all alsa-lib patches :)

I'm for removing aserver & co, too, but let's put it as post-1.0.29
item, since Jaroslav seems preparing 1.0.29 release now.


thanks,

Takashi

Post by Alexander E. Patrakov

Post by Renu Tyagi
Bug and Patch description
1. Changed file : aserver.c
Socket not closed before returning when bind fails
if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
return result;
}
return sock;
}
if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
close(sock);
return result;
}
return sock;
}
2.Changed file : control_shm.c
Socket not closed before returning when connect fails
if (connect(sock, (struct sockaddr *) addr, size) < 0)
return -errno;
return sock;
}
if (connect(sock, (struct sockaddr *) addr, size) < 0){
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}
3.Changed file : pcm_shm.c
Socket not closed before returning when connect fails
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
return -errno;
}
return sock;
}
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}
PFA patch.
Thanks & Regards,
Renu Tyagi
_______________________________________________
Alsa-devel mailing list
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

--
Alexander E. Patrakov

[Takashi Iwai]

At Mon, 15 Sep 2014 13:48:11 +0200,

Post by Takashi Iwai
At Mon, 15 Sep 2014 17:36:02 +0600,

Post by Alexander E. Patrakov

Post by Renu Tyagi
Hi,
I ran Coverity analysis tool on alsa and found some bugs.

May I suggest that we remove aserver and the shm plugin instead of
applying the patch? Three days ago I tried to use it for testing my fix
to the share plugin, but failed. In other words: if even speaker-test
cannot be made to work on it without crashing and/or hanging or valgrind
errors, then I'd rather be aggressive here.
And next time please CC: Takashi Iwai on all alsa-lib patches :)

I'm for removing aserver & co, too, but let's put it as post-1.0.29
item, since Jaroslav seems preparing 1.0.29 release now.

BTW, I haven't received the original mail from ML server.
(The delivery seems slowed down by some reason, so it might be that,
though...)


Takashi

[Takashi Iwai]

At Tue, 16 Sep 2014 04:08:48 +0000 (GMT),

Hi Takashi,
Here are some more bugs and patches

Could you post one patch per mail? This would make it much easier to
review.


thanks,

Takashi

1. File in which changes are being made : conf.c
Value of err to be Negative for correct error handling
Patch : 22657.patch
if (err >= 0) {
snd_config_iterator_t i, next;
if (snd_config_get_type(func_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for func %s definition", str);
goto _err;
}
if (err >= 0) {
snd_config_iterator_t i, next;
if (snd_config_get_type(func_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for func %s definition", str);
err = -EINVAL;
goto _err;
}
----------------------------------------------------------------------------------------
2.File in which changes are being made : control.c
Value of err to be Negative for correct error handling
Patch : 22658.patch
if (err >= 0) {
if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for CTL type %s definition", str);
goto _err;
}
if (err >= 0) {
if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for CTL type %s definition", str);
err = -EINVAL;
goto _err;
}
----------------------------------------------------------------------------------------
3.File in which changes are being made : mixer.c
Double free by calling snd_hctl_close(hctl) twice it is called inside snd_mixer_attach_hctl also.
Patch : 22638.patch
int snd_mixer_attach(snd_mixer_t *mixer, const char *name)
{
snd_hctl_t *hctl;
int err;
err = snd_hctl_open(&hctl, name, 0);
if (err < 0)
return err;
err = snd_mixer_attach_hctl(mixer, hctl);
if (err < 0) {
snd_hctl_close(hctl);
return err;
}
return 0;
}
int snd_mixer_attach(snd_mixer_t *mixer, const char *name)
{
snd_hctl_t *hctl;
int err;
err = snd_hctl_open(&hctl, name, 0);
if (err < 0)
return err;
err = snd_mixer_attach_hctl(mixer, hctl);
if (err < 0) {
/* Removed snd_hctl_close(hctl) to avoid double free, already called in snd_mixer_attach_hctl. */
return err;
}
return 0;
}
----------------------------------------------------------------------------------------
4.File in which changes are being made : pcm.c
Value of err to be Negative for correct error handling
Patch : 22659.patch
if (err >= 0) {
if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for PCM type %s definition", str);
goto _err;
}
snd_config_for_each(i, next, type_conf) {
snd_config_t *n = snd_config_iterator_entry(i);
if (err >= 0) {
if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for PCM type %s definition", str);
err = -EINVAL;
goto _err;
}
snd_config_for_each(i, next, type_conf) {
snd_config_t *n = snd_config_iterator_entry(i);
----------------------------------------------------------------------------------------
5.File in which changes are being made : pcm_file.c
Free resources before returning error.
Patch : 22583.patch
Patch : 22584.patch
if (ifname && (stream == SND_PCM_STREAM_CAPTURE)) {
ifd = open(ifname, O_RDONLY); /* TODO: mind blocking mode */
if (ifd < 0) {
SYSERR("open %s for reading failed", ifname);
free(file);
return -errno;
}
file->ifname = strdup(ifname);
}
file->fd = fd;
file->ifd = ifd;
file->format = format;
file->gen.slave = slave;
file->gen.close_slave = close_slave;
err = snd_pcm_new(&pcm, SND_PCM_TYPE_FILE, name, slave->stream, slave->mode);
if (err < 0) {
free(file->fname);
free(file);
return err;
}
if (ifname && (stream == SND_PCM_STREAM_CAPTURE)) {
ifd = open(ifname, O_RDONLY); /* TODO: mind blocking mode */
if (ifd < 0) {
SYSERR("open %s for reading failed", ifname);
free(file->fname);
free(file);
return -errno;
}
file->ifname = strdup(ifname);
}
file->fd = fd;
file->ifd = ifd;
file->format = format;
file->gen.slave = slave;
file->gen.close_slave = close_slave;
err = snd_pcm_new(&pcm, SND_PCM_TYPE_FILE, name, slave->stream, slave->mode);
if (err < 0) {
free(file->fname);
free(file->ifname);
free(file);
return err;
}
----------------------------------------------------------------------------------------
6.File in which changes are being made : pcm_hooks.c
Null check before closing h
Patch :22665.patch
if (err >= 0)
err = hook_add_dlobj(pcm, h);
if (err < 0) {
snd_dlclose(h);
return err;
}
return 0;
}
if (err >= 0)
err = hook_add_dlobj(pcm, h);
if (err < 0) {
if(h)
snd_dlclose(h);
return err;
}
return 0;
}
----------------------------------------------------------------------------------------
7.File in which changes are being made : pcm_share.c
Mutex unlock before returning
Patch : 22670.patch
Pthread_mutex_lock(&slave->mutex);
err = pipe(slave->poll);
if (err < 0) {
SYSERR("can't create a pipe");
return NULL;
}
while (slave->open_count > 0) {
snd_pcm_uframes_t missing;
// printf("begin min_missing\n");
missing = _snd_pcm_share_slave_missing(slave);
// printf("min_missing=%ld\n", missing);
if (missing < INT_MAX) {
snd_pcm_uframes_t hw_ptr;
snd_pcm_sframes_t avail_min;
hw_ptr = slave->hw_ptr + missing;
hw_ptr += spcm->period_size - 1;
if (hw_ptr >= spcm->boundary)
hw_ptr -= spcm->boundary;
hw_ptr -= hw_ptr % spcm->period_size;
avail_min = hw_ptr - *spcm->appl.ptr;
if (spcm->stream == SND_PCM_STREAM_PLAYBACK)
avail_min += spcm->buffer_size;
if (avail_min < 0)
avail_min += spcm->boundary;
// printf("avail_min=%d\n", avail_min);
if ((snd_pcm_uframes_t)avail_min != spcm->avail_min) {
snd_pcm_sw_params_set_avail_min(spcm, &slave->sw_params, avail_min);
err = snd_pcm_sw_params(spcm, &slave->sw_params);
if (err < 0) {
SYSERR("snd_pcm_sw_params error");
return NULL;
}
}
Pthread_mutex_lock(&slave->mutex);
err = pipe(slave->poll);
if (err < 0) {
SYSERR("can't create a pipe");
Pthread_mutex_unlock(&slave->mutex);
return NULL;
}
while (slave->open_count > 0) {
snd_pcm_uframes_t missing;
// printf("begin min_missing\n");
missing = _snd_pcm_share_slave_missing(slave);
// printf("min_missing=%ld\n", missing);
if (missing < INT_MAX) {
snd_pcm_uframes_t hw_ptr;
snd_pcm_sframes_t avail_min;
hw_ptr = slave->hw_ptr + missing;
hw_ptr += spcm->period_size - 1;
if (hw_ptr >= spcm->boundary)
hw_ptr -= spcm->boundary;
hw_ptr -= hw_ptr % spcm->period_size;
avail_min = hw_ptr - *spcm->appl.ptr;
if (spcm->stream == SND_PCM_STREAM_PLAYBACK)
avail_min += spcm->buffer_size;
if (avail_min < 0)
avail_min += spcm->boundary;
// printf("avail_min=%d\n", avail_min);
if ((snd_pcm_uframes_t)avail_min != spcm->avail_min) {
snd_pcm_sw_params_set_avail_min(spcm, &slave->sw_params, avail_min);
err = snd_pcm_sw_params(spcm, &slave->sw_params);
if (err < 0) {
SYSERR("snd_pcm_sw_params error");
Pthread_mutex_unlock(&slave->mutex);
return NULL;
}
}
----------------------------------------------------------------------------------------
8.File in which changes are being made : rawmidi.c
Handle h to be closed in case of error before returning
Patch :22578.patch
if (err >= 0)
err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
if (err < 0)
return err;
if (inputp) {
(*inputp)->dl_handle = h; h = NULL;
if (err >= 0)
err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
if (err < 0){
if (h)
snd_dlclose(h);
return err;
}
if (inputp) {
(*inputp)->dl_handle = h; h = NULL;
----------------------------------------------------------------------------------------
9.File in which changes are being made : sbase.c
Freeing the resources before returning in case of error
Patch : 22579.patch
hsimple = calloc(1, sizeof(*hsimple));
if (hsimple == NULL) {
snd_mixer_selem_id_free(id);
return -ENOMEM;
}
switch (sel->purpose) {
if (ctype != SND_CTL_ELEM_TYPE_BOOLEAN) {
snd_mixer_selem_id_free(id);
return -EINVAL;
}
break;
hsimple = calloc(1, sizeof(*hsimple));
if (hsimple == NULL) {
snd_mixer_selem_id_free(id);
return -ENOMEM;
}
switch (sel->purpose) {
if (ctype != SND_CTL_ELEM_TYPE_BOOLEAN) {
snd_mixer_selem_id_free(id);
free(hsimple);
return -EINVAL;
}
break;
---------------------------------------------------------------------------
10.File in which changes are being made : socket.c
Socket not closed before returning when error occurs
Patch :22587.patch
s = socket(PF_INET, SOCK_STREAM, 0);
if (s < 0) {
SYSERR("socket failed");
return -errno;
}
conf.ifc_len = numreqs * sizeof(struct ifreq);
conf.ifc_buf = malloc((unsigned int) conf.ifc_len);
if (! conf.ifc_buf)
return -ENOMEM;
while (1) {
err = ioctl(s, SIOCGIFCONF, &conf);
if (err < 0) {
SYSERR("SIOCGIFCONF failed");
return -errno;
}
if ((size_t)conf.ifc_len < numreqs * sizeof(struct ifreq))
break;
numreqs *= 2;
conf.ifc_len = numreqs * sizeof(struct ifreq);
conf.ifc_buf = realloc(conf.ifc_buf, (unsigned int) conf.ifc_len);
if (! conf.ifc_buf)
return -ENOMEM;
}
s = socket(PF_INET, SOCK_STREAM, 0);
if (s < 0) {
SYSERR("socket failed");
return -errno;
}
conf.ifc_len = numreqs * sizeof(struct ifreq);
conf.ifc_buf = malloc((unsigned int) conf.ifc_len);
if (! conf.ifc_buf){
close(s);
return -ENOMEM;
}
while (1) {
err = ioctl(s, SIOCGIFCONF, &conf);
if (err < 0) {
SYSERR("SIOCGIFCONF failed");
close(s);
return -errno;
}
if ((size_t)conf.ifc_len < numreqs * sizeof(struct ifreq))
break;
numreqs *= 2;
conf.ifc_len = numreqs * sizeof(struct ifreq);
conf.ifc_buf = realloc(conf.ifc_buf, (unsigned int) conf.ifc_len);
if (! conf.ifc_buf){
close(s);
return -ENOMEM;
}
}
----------------------------------------------------------------------
11.File in which changes are being made : simple_abst.c
If lib is NULL return error to avoid strlen of NULL string.
Patch :22667.patch
path = getenv("ALSA_MIXER_SIMPLE_MODULES");
if (!path)
path = SO_PATH;
xlib = malloc(strlen(lib) + strlen(path) + 1 + 1);
if (xlib == NULL)
return -ENOMEM;
path = getenv("ALSA_MIXER_SIMPLE_MODULES");
if (!lib)
return -ENXIO;
if (!path)
path = SO_PATH;
xlib = malloc(strlen(lib) + strlen(path) + 1 + 1);
if (xlib == NULL)
return -ENOMEM;
------------------------------------------------------------------------------
------- Original Message -------
Date : Sep 15, 2014 17:18 (GMT+05:30)
Title : Re: [alsa-devel] coverity fix in alsa-libs
At Mon, 15 Sep 2014 17:36:02 +0600,

Post by Alexander E. Patrakov

Post by Renu Tyagi
Hi,
I ran Coverity analysis tool on alsa and found some bugs.

May I suggest that we remove aserver and the shm plugin instead of
applying the patch? Three days ago I tried to use it for testing my fix
to the share plugin, but failed. In other words: if even speaker-test
cannot be made to work on it without crashing and/or hanging or valgrind
errors, then I'd rather be aggressive here.
And next time please CC: Takashi Iwai on all alsa-lib patches :)

I'm for removing aserver & co, too, but let's put it as post-1.0.29
item, since Jaroslav seems preparing 1.0.29 release now.
thanks,
Takashi

Post by Alexander E. Patrakov

Post by Renu Tyagi
Bug and Patch description
1. Changed file : aserver.c
Socket not closed before returning when bind fails
if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
return result;
}
return sock;
}
if (bind(sock, (struct sockaddr *) addr, size) < 0) {
int result = -errno;
SYSERROR("bind failed");
close(sock);
return result;
}
return sock;
}
2.Changed file : control_shm.c
Socket not closed before returning when connect fails
if (connect(sock, (struct sockaddr *) addr, size) < 0)
return -errno;
return sock;
}
if (connect(sock, (struct sockaddr *) addr, size) < 0){
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}
3.Changed file : pcm_shm.c
Socket not closed before returning when connect fails
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
return -errno;
}
return sock;
}
if (connect(sock, (struct sockaddr *) addr, size) < 0) {
SYSERR("connect failed");
close(sock);
return -errno;
}
return sock;
}
PFA patch.
Thanks & Regards,
Renu Tyagi
_______________________________________________
Alsa-devel mailing list
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

--
Alexander E. Patrakov

_______________________________________________
Alsa-devel mailing list
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Thanks & Regards,
Renu Tyagi
SRI-Delhi (SW Platform) | Seat Number 3A-157
Samsung India Electronics Pvt. Ltd.,
2A, Sector 126, Noida -201303
[2 22657.patch <application/octet-stream (base64)>]
[3 22658.patch <application/octet-stream (base64)>]
[4 22638.patch <application/octet-stream (base64)>]
[5 22659.patch <application/octet-stream (base64)>]
[6 22583.patch <application/octet-stream (base64)>]
[7 22584.patch <application/octet-stream (base64)>]
[8 22665.patch <application/octet-stream (base64)>]
[9 22670.patch <application/octet-stream (base64)>]
[10 22578.patch <application/octet-stream (base64)>]
[11 22579.patch <application/octet-stream (base64)>]
[12 22587.patch <application/octet-stream (base64)>]
[13 22667.patch <application/octet-stream (base64)>]

[Takashi Iwai]

At Wed, 17 Sep 2014 03:17:08 +0000 (GMT),

Hi Takashi,
File in which changes are being made : rawmidi.c
Bug type - Handle h to be closed in case of error before returning
PFA patch.

By some reason, your post doesn't appear on alsa-devel ML.
Did you subscribe the ML? If not, please subscribe and repost.
Also, what about other patches you made?

Last but not least: please prepare a patch to be applicable via
git-am, including subject and from tags and the changelog content, and
don't forget your sign-off to the patch, just like a patch to Linux
kernel.


thanks,

Takashi

if (err >= 0)
err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
if (err < 0)
return err;
if (inputp) {
(*inputp)->dl_handle = h; h = NULL;
if (err >= 0)
err = open_func(inputp, outputp, name, rawmidi_root, rawmidi_conf, mode);
if (err < 0){
if (h)
snd_dlclose(h);
return err;
}
if (inputp) {
(*inputp)->dl_handle = h; h = NULL;
Thanks & Regards,
Renu Tyagi
[2 22578.patch <application/octet-stream (base64)>]

[Renu Tyagi]

Hi,

Sorry for the spam mail. I am sending the mail in plain text now.
PFA patch.
File in which changes are being made : rawmidi.c
Bug type - Handle h to be closed in case of error before returning
Meanwhile I am looking into the matter why my mails are not visible in ML.

Thanks
Renu Tyagi

[Takashi Iwai]

At Thu, 18 Sep 2014 03:57:36 +0000 (GMT),

Post by Renu Tyagi
Hi,
Sorry for the spam mail. I am sending the mail in plain text now.
PFA patch.
File in which changes are being made : rawmidi.c
Bug type - Handle h to be closed in case of error before returning
Meanwhile I am looking into the matter why my mails are not visible in ML.

I see now on ML, so the problem was your post in HTML, indeed.

Regarding the patch: I see only a few cosmetic issue.

- Use embedded in the mail as much as possible rather than an
attachment. This makes review much easier.

- Put your real name in sign-off-by (and also From: line). Refer to
Documentation/SubmittingPatches in Linux kernel tree about the
meaning of this line.

- Put a space before the open brace.

- Add some prefix in the subject line to identify the area; in this
case, add like "rawmidi: Handle d to be ...."


thanks,

Takashi

[Renu Tyagi]

Hi,
I have done the cosmetic changes.
I dont trust my mailers editor so I am attaching it as well :)

Thanks
Renu Tyagi
-----------------------------------------
From 66bbcc5e532c24876bd2fa2bc9d86609679e1062 Mon Sep 17 00:00:00 2001
From: renu tyagi <***@samsung.com>
Date: Thu, 18 Sep 2014 13:23:51 +0530
Subject: [PATCH] rawmidi : Handle h to be closed in case of error before returning


Signed-off-by: renu tyagi <***@samsung.com>
---
src/rawmidi/rawmidi.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c
index b835b47..ac699b4 100644
--- a/src/rawmidi/rawmidi.c
+++ b/src/rawmidi/rawmidi.c
@@ -256,8 +256,11 @@ 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) {
+ if (h)
+ snd_dlclose(h);
return err;
+ }
if (inputp) {
(*inputp)->dl_handle = h; h = NULL;
snd_rawmidi_params_default(*inputp, &params);

[Takashi Iwai]

At Thu, 18 Sep 2014 08:06:18 +0000 (GMT),

Post by Renu Tyagi
Hi,
I have done the cosmetic changes.
I dont trust my mailers editor so I am attaching it as well :)

Unfortunately your untrusted mailer didn't serve as wished.

The patch itself looks good. Could you post the whole series?


thanks,

Takashi

Post by Renu Tyagi
Thanks
Renu Tyagi
-----------------------------------------
From 66bbcc5e532c24876bd2fa2bc9d86609679e1062 Mon Sep 17 00:00:00 2001
Date: Thu, 18 Sep 2014 13:23:51 +0530
Subject: [PATCH] rawmidi : Handle h to be closed in case of error before returning
---
src/rawmidi/rawmidi.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c
index b835b47..ac699b4 100644
--- a/src/rawmidi/rawmidi.c
+++ b/src/rawmidi/rawmidi.c
@@ -256,8 +256,11 @@ 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) {
+ if (h)
+ snd_dlclose(h);
return err;
+ }
if (inputp) {
(*inputp)->dl_handle = h; h = NULL;
snd_rawmidi_params_default(*inputp, &params);
--
1.7.1

[Renu Tyagi]

Hi
Bug type : return negative value in case of error
I caught this error while running coverity on a older version. coverity will not give issue with latest version
But As per my analysis negative error should be returned in these cases, for debugging later on.
File changed : conf.c, control.c , pcm.c
PFA patch
-------------------------------------------------------------------------------------------
From d545d1ebedfbc42041e163536fbe2a6bd43361a0 Mon Sep 17 00:00:00 2001
From: renu tyagi <***@samsung.com>
Date: Thu, 18 Sep 2014 16:22:31 +0530
Subject: [PATCH 3/8] [conf, control, pcm] return negative value in case of error


Signed-off-by: renu tyagi <***@samsung.com>
---
src/conf.c | 1 +
src/control/control.c | 1 +
src/pcm/pcm.c | 1 +
3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/src/conf.c b/src/conf.c
index 5ccc8e1..bb256e7 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -4198,6 +4198,7 @@ static int _snd_config_evaluate(snd_config_t *src,
snd_config_iterator_t i, next;
if (snd_config_get_type(func_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for func %s definition", str);
+ err = -EINVAL;
goto _err;
}
snd_config_for_each(i, next, func_conf) {
diff --git a/src/control/control.c b/src/control/control.c
index d66ed75..dd428a1 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -864,6 +864,7 @@ static int snd_ctl_open_conf(snd_ctl_t **ctlp, const char *name,
if (err >= 0) {
if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for CTL type %s definition", str);
+ err = -EINVAL;
goto _err;
}
snd_config_for_each(i, next, type_conf) {
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 1399a5b..79359dc 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -2143,6 +2143,7 @@ static int snd_pcm_open_conf(snd_pcm_t **pcmp, const char *name,
if (err >= 0) {
if (snd_config_get_type(type_conf) != SND_CONFIG_TYPE_COMPOUND) {
SNDERR("Invalid type for PCM type %s definition", str);
+ err = -EINVAL;
goto _err;
}
snd_config_for_each(i, next, type_conf) {

[Takashi Iwai]

At Thu, 18 Sep 2014 11:15:50 +0000 (GMT),

Hi,
Bug type : missing freeing resources before returning in case of errors
Files changed : sbase.c pcm_file.c
PFA patch
-------------------------------------------------------------------
From f201df1f5c5d129f21c663c81fdb495d9a3ca51e Mon Sep 17 00:00:00 2001
Date: Thu, 18 Sep 2014 16:17:55 +0530
Subject: [PATCH 2/8] [sbase , pcm_file] : freeing resources before returning in case of errors

Please just send the patch as is. We don't need any foreword.
Also, if the changes cover over different areas, split the patch.


thanks,

Takashi

---
modules/mixer/simple/sbase.c | 1 +
src/pcm/pcm_file.c | 2 ++
2 files changed, 3 insertions(+), 0 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) {
snd_mixer_selem_id_free(id);
+ free(hsimple);
return -EINVAL;
}
break;
diff --git a/src/pcm/pcm_file.c b/src/pcm/pcm_file.c
index a0b8bf4..f6d222f 100644
--- a/src/pcm/pcm_file.c
+++ b/src/pcm/pcm_file.c
@@ -758,6 +758,7 @@ int snd_pcm_file_open(snd_pcm_t **pcmp, const char *name,
ifd = open(ifname, O_RDONLY); /* TODO: mind blocking mode */
if (ifd < 0) {
SYSERR("open %s for reading failed", ifname);
+ free(file->fname);
free(file);
return -errno;
}
@@ -772,6 +773,7 @@ int snd_pcm_file_open(snd_pcm_t **pcmp, const char *name,
err = snd_pcm_new(&pcm, SND_PCM_TYPE_FILE, name, slave->stream, slave->mode);
if (err < 0) {
free(file->fname);
+ free(file->ifname);
free(file);
return err;
}
--
1.7.1
Thanks
Renu Tyagi