At Thu, 27 Nov 2008 15:01:51 +0100 (CET),
Julia Lawall wrote:
>
> On Thu, 27 Nov 2008, Takashi Iwai wrote:
>
> > At Thu, 27 Nov 2008 14:33:30 +0100 (CET),
> > Julia Lawall wrote:
> > >
> > > Oops, please ignore this patch. There is a problem, but the modification
> > > is not correct.
> >
> > How about the patch below?
>
> My solution was to move the initialization of mgr->chip[idx] down below
> the last error return in snd_mixart_create. I think it depends on what is
> going to happen to the structure ops. In your solution, if ops.dev_free
> is used later on, then the link between mgr and card will be broken, and
> no one will know to free card.
>
> There is the same code and the same problem in sound/pci/pcxhr/pcxhr.c
>
> I attach a proposed patch for both below.
Looks good. Could you repost with a readable comment?
thanks,
Takashi
>
> julia
>
> > Takashi
> >
> > ---
> > diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c
> > index ae7601f..3cccfed 100644
> > --- a/sound/pci/mixart/mixart.c
> > +++ b/sound/pci/mixart/mixart.c
> > @@ -989,6 +989,7 @@ static int snd_mixart_pcm_digital(struct snd_mixart *chip)
> >
> > static int snd_mixart_chip_free(struct snd_mixart *chip)
> > {
> > + chip->mgr->chip[chip->chip_idx] = NULL;
> > kfree(chip);
> > return 0;
> > }
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> ---
> sound/pci/mixart/mixart.c | 4 +++-
> sound/pci/pcxhr/pcxhr.c | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c
> index ae7601f..f23a735 100644
> --- a/sound/pci/mixart/mixart.c
> +++ b/sound/pci/mixart/mixart.c
> @@ -1010,7 +1010,7 @@ static int __devinit snd_mixart_create(struct mixart_mgr *mgr, struct snd_card *
> .dev_free = snd_mixart_chip_dev_free,
> };
>
> - mgr->chip[idx] = chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> + chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> if (! chip) {
> snd_printk(KERN_ERR "cannot allocate chip\n");
> return -ENOMEM;
> @@ -1025,6 +1025,7 @@ static int __devinit snd_mixart_create(struct mixart_mgr *mgr, struct snd_card *
> return err;
> }
>
> + mgr->chip[idx] = chip;
> snd_card_set_dev(card, &mgr->pci->dev);
>
> return 0;
> @@ -1377,6 +1378,7 @@ static int __devinit snd_mixart_probe(struct pci_dev *pci,
> sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
>
> if ((err = snd_mixart_create(mgr, card, i)) < 0) {
> + snd_card_free(card);
> snd_mixart_free(mgr);
> return err;
> }
> diff --git a/sound/pci/pcxhr/pcxhr.c b/sound/pci/pcxhr/pcxhr.c
> index 73de6e9..7d2b136 100644
> --- a/sound/pci/pcxhr/pcxhr.c
> +++ b/sound/pci/pcxhr/pcxhr.c
> @@ -1024,7 +1024,7 @@ static int __devinit pcxhr_create(struct pcxhr_mgr *mgr, struct snd_card *card,
> .dev_free = pcxhr_chip_dev_free,
> };
>
> - mgr->chip[idx] = chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> + chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> if (! chip) {
> snd_printk(KERN_ERR "cannot allocate chip\n");
> return -ENOMEM;
> @@ -1050,6 +1050,7 @@ static int __devinit pcxhr_create(struct pcxhr_mgr *mgr, struct snd_card *card,
> return err;
> }
>
> + mgr->chip[idx] = chip;
> snd_card_set_dev(card, &mgr->pci->dev);
>
> return 0;
> @@ -1310,6 +1311,7 @@ static int __devinit pcxhr_probe(struct pci_dev *pci, const struct pci_device_id
> sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
>
> if ((err = pcxhr_create(mgr, card, i)) < 0) {
> + snd_card_free(card);
> pcxhr_free(mgr);
> return err;
> }
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
