linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Andreas Dilger <adilger@dilger.ca>
Cc: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ext4: remove unnecessary gotos in ext4_xattr_set_entry
Date: Sat, 1 Jun 2019 20:15:15 -0400	[thread overview]
Message-ID: <20190602001515.GB15741@mit.edu> (raw)
In-Reply-To: <01FB1966-0466-4AB2-913B-F53E8CA816BE@dilger.ca>

On Fri, May 31, 2019 at 03:46:54PM -0600, Andreas Dilger wrote:
> On May 31, 2019, at 6:10 AM, Pavel Tikhomirov <ptikhomirov@virtuozzo.com> wrote:
> > 
> > In the "out" label we only iput old/new_ea_inode-s, in all these places
> > these variables are always NULL so there is no point in goto to "out".
> > 
> > Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> 
> I'm not a fan of changes like this, since it adds potential complexity/bugs
> if the error handling path is changed in the future.  That is one of the major
> benefits of the "goto out_*" model of error handling is that you only need to
> add one new label to the end of the function when some new state is added that
> needs to be cleaned up, compared to having to check each individual error to
> see if something needs to be cleaned up.

I'm not a fan either, for the reasons Andreas stated; if you ever move
code around, it's much more hazardous because you now have to check if
what had previously been a "return ret" now has to change into "goto
outl".  In some case, it's really obvious, if the code is at the very
beginning of the function, but when you're 35 lines down, well over
the size of many of an editor window, it's no longer quite so obvious
whether or not "goto out" is necessary.

						- Ted

      reply	other threads:[~2019-06-02  0:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31 12:10 [PATCH] ext4: remove unnecessary gotos in ext4_xattr_set_entry Pavel Tikhomirov
2019-05-31 21:46 ` Andreas Dilger
2019-06-02  0:15   ` Theodore Ts'o [this message]

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=20190602001515.GB15741@mit.edu \
    --to=tytso@mit.edu \
    --cc=adilger@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ptikhomirov@virtuozzo.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;
as well as URLs for NNTP newsgroup(s).