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@vger.kernel.org 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;
  

  }