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
Date: Tue, 1 Dec 2015 11:00:45 +0300 [thread overview]
Message-ID: <20151201080045.GE18797@mwanda> (raw)
In-Reply-To: <1448950555-8846-1-git-send-email-sudipm.mukherjee@gmail.com>
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
next prev parent reply other threads:[~2015-12-01 8:00 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-01 6:15 [PATCH] staging: unisys: use common return path Sudip Mukherjee
2015-12-01 8:00 ` Dan Carpenter [this message]
2015-12-01 8:06 ` Dan Carpenter
2015-12-01 9:57 ` Sudip Mukherjee
2015-12-01 16:05 ` Dan Carpenter
2015-12-02 4:49 ` Sudip Mukherjee
2015-12-01 14:54 ` Ben Romer
2015-12-01 15:57 ` Dan Carpenter
2015-12-01 16:16 ` Ben Romer
2015-12-02 5:01 ` Sudip Mukherjee
2015-12-02 5:02 ` Sudip Mukherjee
2015-12-02 6:20 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151201080045.GE18797@mwanda \
--to=dan.carpenter@oracle.com \
--cc=benjamin.romer@unisys.com \
--cc=david.kershner@unisys.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sparmaintainer@unisys.com \
--cc=sudipm.mukherjee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox