Messages in this thread | | | Subject | Re: [PATCH] staging: unisys: use common return path | From | Ben Romer <> | Date | Tue, 1 Dec 2015 11:16:16 -0500 |
| |
On 12/01/2015 10:57 AM, Dan Carpenter wrote: > What I meant was that I'm generally opposed to "common exit paths". > Mixing all the exit paths together often makes the code more complicated > and leads to errors. That makes sense from a common sense perspective > that doing many things is more difficult than doing one thing? Anyway > it's easy enough to verify empirically that this style is bug prone. > > On the other hand there are times where all exit paths need to unlock or > to free a variable and in those cases using a common exit path makes > sense. Just don't standardize on "Every function should only have a > single return". >
That works for me. Mainly my issue with it is that I've spent a lot of time trying to eliminate "goto Away" code from the drivers, so I'd rather not put any back if possible.
>> >> If we *have* to change it > > I don't think we have to change it at all. Using direct returns makes > finding locking bugs easier for static checkers. >
That's true, and I think the code is fine as it is.
>> spin_unlock_irqrestore(&devdata->priv_lock, flags); > > This is a bug. >
Indeed, but I'd rather not have any of these changes made anyway. This function isn't broken so it doesn't need to be fixed.
-- Ben
| |