[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