On Sun, Dec 19, 2010 at 11:33:42AM +0000, Christopher Brannon wrote: > Dan Carpenter <error27@xxxxxxxxx> 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 devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
