[PATCH] libertas: fix error handling

Marcelo Tosatti marcelo at kvack.org
Tue Feb 27 03:08:59 EST 2007


On Tue, Feb 27, 2007 at 08:47:45AM +0100, Holger Schurig wrote:
> > > -	unregister_netdev(dev);
> > > +		if (dev && dev->reg_state)
> > > +			unregister_netdev(dev);
> 
> > This is a layering violation... Shouldnt poke into
> > dev->reg_state from driver context like this.
> 
> I agree. However, if the release function is called twice, then 
> unregister_netdev() complains. And I saw nothing like 
> is_netdev_registered() (or similar) in 
> include/linux/netdevice.h.

Right, I also looked for that, but there's nothing like it.

> > Undoing initialization steps (error handling) should be done
> > at the initialization sites. Having a single function for both
> > half-initialized and full removal/unregister cases is
> > confusing and bug-prone.
> 
> I agree.
> 
> > Can you please cook a patch to handle failures at
> > initialization instead of this?
> 
> 
> Let's explain the reason why I coded it the way it is:
> 
> The problem is that currently our card can be un-unitialized. I 
> had to break wlan_add_card() into an add-card, set-hw-funcs and 
> start-card functionality, because two of them are 
> hardware-independ and one is not.
> 
> Now, the add-card functionality requests memory. If that wouldn't 
> work, it could tear down that inside itself. That is common 
> practice.
> 
> The set-hw-funcs cannot have an error, no problem here.
> 
> But the start-card functionality CAN produce errors. And it can 
> clean up it's own mess .....  but it cannot call back into the 
> add-card function to clean up the mess there.
> 
> Therefore I thought about about a common clean-up routine that 
> can tidy up everything, no matter what.

I understand... But I think its cleaner and less bugprone if we handle
errors right at the callers, not later by a common function.




More information about the libertas-dev mailing list