Skip to contentSkip to conversation

2.4.18 linux/drivers/maestro.c dev_audio flaw

PATCH In review Samuel Thibault 3 participants 3 messages Jun 30, 2002 – Jul 1, 2002 Sound

lore
Share & tools
lore

Samuel Thibault

author
#0 lore raw

PATCH 1/1 2.4.18 linux/drivers/maestro.c dev_audio flaw

Hi !

It seems that the value of ess->dev_audio is wrongly interpreted. Here's its only initialization :
(maestro.c:3473)

		if ((s->dev_audio = register_sound_dsp(&ess_audio_fops, -1)) < 0)

register_sound_dsp can return -ENOMEM for instance, so checking whether < 0 is mandatory. However, in (almost) the rest of the file, only = -1 is tested, so here's a patch.

Furthermore, printing some error message may sometimes be handy, such as when -ENOENT is returned (no hole was found for the driver), just to understand why the device was probed, but isn't actually accessible.

Best regards,

Samuel Thibault

drivers/sound/maestro.c +7 -7
diff -urN linux-2.4.18-cor/drivers/sound/maestro.c linux-2.4.18-cor2/drivers/sound/maestro.c
@@ -1963,7 +1963,7 @@
 	for(i=0;i<NR_DSPS;i++)
 	{
 		s=&c->channels[i];
-		if(s->dev_audio = -1)
+		if(s->dev_audio < 0)
 			break;
 		spin_lock(&s->lock);
 		ess_update_ptr(s);

@@ -2917,7 +2917,7 @@
 	for(i=0;i<NR_DSPS;i++) {
 		struct ess_state *ess = &s->card->channels[i];
 
-		if(ess->dev_audio = -1)
+		if(ess->dev_audio < 0)
 			continue;
 
 		ess->dma_dac.ready = s->dma_dac.mapped = 0;

@@ -3480,7 +3480,7 @@
 	for(;i<NR_DSPS;i++)
 	{
 		struct ess_state *s=&card->channels[i];
-		s->dev_audio = -1;
+		s->dev_audio = -ENODEV;
 	}
 
 	ess = &card->channels[0];

@@ -3541,7 +3541,7 @@
 		for(i=0;i<NR_DSPS;i++)
 		{
 			struct ess_state *s = &card->channels[i];
-			if(s->dev_audio != -1)
+			if(s->dev_audio >= 0)
 				unregister_sound_dsp(s->dev_audio);
 		}
 		release_region(card->iobase, 256);

@@ -3586,7 +3586,7 @@
 	for(i=0;i<NR_DSPS;i++)
 	{
 		struct ess_state *ess = &card->channels[i];
-		if(ess->dev_audio != -1)
+		if(ess->dev_audio >= 0)
 			unregister_sound_dsp(ess->dev_audio);
 	}
 	/* Goodbye, Mr. Bond. */

@@ -3690,7 +3690,7 @@
 	for(i=0;i<NR_DSPS;i++) {
 		struct ess_state *s = &card->channels[i];
 
-		if(s->dev_audio = -1)
+		if(s->dev_audio < 0)
 			continue;
 
 		M_printk("maestro: stopping apus for device %d\n",i);

@@ -3743,7 +3743,7 @@
 		struct ess_state *s = &card->channels[i];
 		int chan,reg;
 
-		if(s->dev_audio = -1)
+		if(s->dev_audio < 0)
 			continue;
 
 		for(chan = 0 ; chan < 6 ; chan++) {
#1 lore raw
Quoted 1 line

It seems that the value of ess->dev_audio is wrongly interpreted.

Thanks for attacking this driver, Samuel. Your patches have looked good so far. I'm sorry I'm not more involved; its been quite a while since I've futzed about with the maestro(s).

You may want to cc: your fixes to Takashi Iwai <[email protected]> , who actively maintains the ALSA equivalent drivers. He'll have a much greater chance of affecting change than I do :)

- z

#2 lore raw
In reply to Samuel Thibault
Quoted 6 lines · Zach Brown
Quoted 1 line

It seems that the value of ess->dev_audio is wrongly interpreted.

Thanks for attacking this driver, Samuel. Your patches have looked good so far. I'm sorry I'm not more involved; its been quite a while since I've futzed about with the maestro(s).

I'm afraid I missed the original patches :(

But I am willing to review and apply drivers/sound [sound/oss in 2.5] patches, if no one else (Alan?) does so.

Regards,

Jeff