Re: [PATCH] staging: unisys: use common return path

From: Ben Romer
Date: Tue Dec 01 2015 - 11:19:39 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/