On Sun, Dec 19, 2010 at 11:33:42AM +0000, Christopher Brannon wrote:
> Dan Carpenter <[email protected]> writes:
>
> > I'm not a big fan of the global variables approach to unwinding... It
> > makes reading the code much harder.
>
> What's the best way to do this, while limiting duplication?
> Basically, we have to solve the same problem in two places: unsuccessful
> initialization and successful termination.
Duplication is Ok. The original resource_cleanup() was obviously much
simpler, I don't think anyone could disagree. The controversial bit
that I would argue, is that the original unwind code was much simpler as
well.
When I'm reading through the new resource_cleanup() then I have to
wonder if it's Ok to call synth_release() before it's been aquired. (It
is Ok.) But if you just unwind at the end of the function then you
only call synth_release() when it's needed, so that question doesn't
arise.
You could also have called speakup_unregister_devsynth() unconditionally
but instead the patch introduces speakup_devsynth_is_registered().
Another option would have been to introduce another variable to track
it. But all of those options are kind of ugly, spaghetti code...
If you went with a traditional unwind instead you could remove:
1) All the new variable declarations.
2) The places which say: virtual_keyboard_registered = true;
3) All the if conditions in resource_cleanup()
So sure, it's duplicative, but it's fewer lines of code overall and it's
simpler to read.
regards,
dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
