[bug] pnp_register_card_driver/pnp_unregister_card_driver

[bug] pnp_register_card_driver/pnp_unregister_card_driver

[matthieu castet]

Hi,

pnp_register_driver could fail and return <0 result, in this case the 
driver shouldn't be pnp_unregister_driver.

But if you look in pnp_register_card_driver, the result isn't checked. 
And it is always pnp_unregister_driver in pnp_unregister_card_driver.

I know that pnp_register_card_driver shouldn't fail in normal condition, 
but who know...


Matthieu

Re: [bug] pnp_register_card_driver/pnp_unregister_card_driver

[Adam Belay]

On Mon, Feb 07, 2005 at 08:33:47PM +0100, matthieu castet wrote:
> Hi,
> 
> pnp_register_driver could fail and return <0 result, in this case the 
> driver shouldn't be pnp_unregister_driver.
> 
> But if you look in pnp_register_card_driver, the result isn't checked. 
> And it is always pnp_unregister_driver in pnp_unregister_card_driver.
> 
> I know that pnp_register_card_driver shouldn't fail in normal condition, 
> but who know...
> 
> 
> Matthieu

Yeah, you're right.  I'm probably going to do something like this.

--- a/drivers/pnp/card.c	2005-01-20 17:38:02.000000000 -0500
+++ b/drivers/pnp/card.c	2005-02-07 14:53:24.000000000 -0500
@@ -355,10 +355,12 @@
 	drv->link.probe = NULL;
 	drv->link.remove = &card_remove_first;
 
+	if ((count = pnp_register_driver(&drv->link) < 0))
+		return count;
+
 	spin_lock(&pnp_lock);
 	list_add_tail(&drv->global_list, &pnp_card_drivers);
 	spin_unlock(&pnp_lock);
-	pnp_register_driver(&drv->link);
 
 	list_for_each_safe(pos,temp,&pnp_cards){
 		struct pnp_card *card = list_entry(pos, struct pnp_card, global_list);

Thanks,
Adam

Re: [bug] pnp_register_card_driver/pnp_unregister_card_driver

[matthieu castet]

Hi,

Adam Belay wrote:
> On Mon, Feb 07, 2005 at 08:33:47PM +0100, matthieu castet wrote:
> 
>>Hi,
>>
>>pnp_register_driver could fail and return <0 result, in this case the 
>>driver shouldn't be pnp_unregister_driver.
>>
>>But if you look in pnp_register_card_driver, the result isn't checked. 
>>And it is always pnp_unregister_driver in pnp_unregister_card_driver.
>>
>>I know that pnp_register_card_driver shouldn't fail in normal condition, 
>>but who know...
>>
>>
>>Matthieu
> 
> 
> Yeah, you're right.  I'm probably going to do something like this.
> 
If you do something like this there will be some broken driver to 
correct. See [1] for isa alsa driver that assume that the result is >=0.


Matthieu

[1]
$grep -rI pnp_register_card_driver /usr/src/linux/sound/isa/
/usr/src/linux/sound/isa/cs423x/cs4236.c:       cards += 
pnp_register_card_driver(&cs423x_pnpc_driver);
/usr/src/linux/sound/isa/gus/interwave.c:       i = 
pnp_register_card_driver(&interwave_pnpc_driver);
/usr/src/linux/sound/isa/sb/es968.c:    int cards = 
pnp_register_card_driver(&es968_pnpc_driver);
/usr/src/linux/sound/isa/sb/sb16.c:     i = 
pnp_register_card_driver(&sb16_pnpc_driver);
/usr/src/linux/sound/isa/als100.c:      cards += 
pnp_register_card_driver(&als100_pnpc_driver);
/usr/src/linux/sound/isa/sscape.c:              ret = 
pnp_register_card_driver(&sscape_pnpc_driver);
/usr/src/linux/sound/isa/opti9xx/opti92x-ad1848.c:      cards = 
pnp_register_card_driver(&opti9xx_pnpc_driver);
/usr/src/linux/sound/isa/ad1816a/ad1816a.c:     cards += 
pnp_register_card_driver(&ad1816a_pnpc_driver);
/usr/src/linux/sound/isa/wavefront/wavefront.c: cards += 
pnp_register_card_driver(&wavefront_pnpc_driver);
/usr/src/linux/sound/isa/dt019x.c:      cards += 
pnp_register_card_driver(&dt019x_pnpc_driver);
/usr/src/linux/sound/isa/es18xx.c:      i = 
pnp_register_card_driver(&es18xx_pnpc_driver);
/usr/src/linux/sound/isa/azt2320.c:     cards += 
pnp_register_card_driver(&azt2320_pnpc_driver);
/usr/src/linux/sound/isa/opl3sa2.c:     cards += 
pnp_register_card_driver(&opl3sa2_pnpc_driver);
/usr/src/linux/sound/isa/cmi8330.c:     cards += 
pnp_register_card_driver(&cmi8330_pnpc_driver);