At Tue, 7 Aug 2007 16:40:48 +0800,
Eugene Teo wrote:
>
> This patch fixes:
> 1) a resource leak (CID: 1817)
> 2) various code cleanups
>
> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]>
> ---
> sound/core/seq/oss/seq_oss_init.c | 29 ++++++++++++++++++-----------
> sound/core/seq/oss/seq_oss_writeq.c | 6 ++++--
> 2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/sound/core/seq/oss/seq_oss_init.c
> b/sound/core/seq/oss/seq_oss_init.c
> index ca5a2ed..c9b95c3 100644
> --- a/sound/core/seq/oss/seq_oss_init.c
> +++ b/sound/core/seq/oss/seq_oss_init.c
> @@ -176,7 +176,8 @@ snd_seq_oss_open(struct file *file, int level)
> int i, rc;
> struct seq_oss_devinfo *dp;
>
> - if ((dp = kzalloc(sizeof(*dp), GFP_KERNEL)) == NULL) {
> + dp = kzalloc(sizeof(*dp), GFP_KERNEL);
> + if (dp == NULL) {
Better to use "if (!dp)"
> snd_printk(KERN_ERR "can't malloc device info\n");
> return -ENOMEM;
> }
> @@ -188,8 +189,8 @@ snd_seq_oss_open(struct file *file, int level)
> }
> if (i >= SNDRV_SEQ_OSS_MAX_CLIENTS) {
> snd_printk(KERN_ERR "too many applications\n");
> - kfree(dp);
> - return -ENOMEM;
> + rc = -ENOMEM;
> + goto _error;
This seems wrong. It goes before initializing dp->queue = -1, so it
screws up delete_seq_queue().
> @@ -276,11 +281,13 @@ snd_seq_oss_open(struct file *file, int level)
> return 0;
>
> _error:
> - snd_seq_oss_synth_cleanup(dp);
> - snd_seq_oss_midi_cleanup(dp);
> - i = dp->queue;
> + snd_seq_oss_writeq_delete(dp->writeq);
> + snd_seq_oss_readq_delete(dp->readq);
> + delete_seq_queue(dp->queue);
> delete_port(dp);
> - delete_seq_queue(i);
> + snd_seq_oss_midi_cleanup(dp);
> + snd_seq_oss_synth_cleanup(dp);
> + kfree(dp);
The order of these calls is important. Did you test it and confirm
that it has no side effects?
Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
