Date: Tue, 1 Dec 2015 11:00:45 +0300
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Benjamin Romer <benjamin.romer@unisys.com>,
        David Kershner <david.kershner@unisys.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        devel@driverdev.osuosl.org, sparmaintainer@unisys.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: unisys: use common return path
Message-ID: <20151201080045.GE18797@mwanda>
References: <1448950555-8846-1-git-send-email-sudipm.mukherjee@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1448950555-8846-1-git-send-email-sudipm.mukherjee@gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1152
Lines: 33

Doing One Err style error handling is often a mistake but it's ok here.
Just choose a better label name, though.  "unlock".  I have seen several
bugs caused because people used the label name "err" instead of
"unlock".  Compare these two examples.

	spin_unlock();
err:
	return ret;

Do you see the bug?  Now look at this code:

	spin_unlock();
unlock:
	return ret;

Just from looking at those few lines you can see that the intent was
to unlock but instead it just returns.  In the first example, you need
to scroll to the start of the function and examine the context to see
what "err" was intended to do.  And you're not likely to even be
suspicious of the err label because do nothing labels with ambiguos
label names are very common.

I have seen at least three places where an ambiguously named label was
placed after the spin unlock instead of before, where it was intended.

regards,
dan carpenter

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