From: Ted Ts'o <tytso@mit.edu>
To: Yongqiang Yang <xiaoqiangnk@gmail.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH V6 RESEND 01/15] ext4: add a function which extends a group without checking parameters
Date: Tue, 3 Jan 2012 16:07:48 -0500 [thread overview]
Message-ID: <20120103210748.GE30502@thunk.org> (raw)
In-Reply-To: <1325242812-27005-2-git-send-email-xiaoqiangnk@gmail.com>
On Fri, Dec 30, 2011 at 06:59:58PM +0800, Yongqiang Yang wrote:
> +static int ext4_group_extend_no_check(struct super_block *sb,
> + ext4_fsblk_t o_blocks_count, ext4_grpblk_t add)
I fixed the whitespace here (nit-picky, I know)
> + handle = ext4_journal_start_sb(sb, 3);
> + if (IS_ERR(handle)) {
> + err = PTR_ERR(handle);
> + ext4_warning(sb, "error %d on journal start", err);
> + goto out;
> + }
There's only two calls "goto out", and out: currently is just "return
err;". So I just changed this to be "return err", which is clearer.
I tend to place more imporance for clarity of the code than rigid
rules which say that there should only be one control flow such that
"return err" must only occur once at the end of the function.
> + err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
> + if (err) {
> + ext4_warning(sb, "error %d on journal write access", err);
> + ext4_journal_stop(handle);
> + goto out;
> + }
Instead of calling ext4_journal_stop() here, it's simpler just to jump
to the "exit_journal" label, which is consistent with what we do
everywhere else. Now all of the error gotos are to "exit_journal",
which I've renamed to "errout".
These are minor stylistic changes, but I thought I would mention them
so that other people understand what is considered the best way to do
things, I've just made these changes myself to avoid asking you to
respin the patches.
Regards,
- Ted
next prev parent reply other threads:[~2012-01-04 3:49 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-30 10:59 [PATCH V6 RESEND] ext4: add new online resize Yongqiang Yang
2011-12-30 10:59 ` [PATCH V6 RESEND 01/15] ext4: add a function which extends a group without checking parameters Yongqiang Yang
2012-01-03 21:07 ` Ted Ts'o [this message]
2012-01-04 3:59 ` Yongqiang Yang
2011-12-30 10:59 ` [PATCH V6 RESEND 02/15] ext4: add a function which adds a new desc to a fs Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 03/15] ext4: add a function which sets up a new group desc Yongqiang Yang
2012-01-04 2:24 ` Ted Ts'o
2012-01-04 4:02 ` Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 04/15] ext4: add a function which updates super block Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 05/15] ext4: add a structure which will be used by 64bit-resize interface Yongqiang Yang
2012-01-04 2:30 ` Ted Ts'o
2011-12-30 11:00 ` [PATCH V6 RESEND 06/15] ext4: add a function which sets up group blocks of a flex groups Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 07/15] ext4: add a function which adds several group descriptors Yongqiang Yang
2012-01-04 2:44 ` Ted Ts'o
2011-12-30 11:00 ` [PATCH V6 RESEND 08/15] ext4: add a function which sets up a flex groups each time Yongqiang Yang
2012-01-04 2:46 ` Ted Ts'o
2011-12-30 11:00 ` [PATCH V6 RESEND 09/15] ext4: enable ext4_update_super() to handle a flex groups Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 10/15] ext4: pass verify_reserved_gdb() the number of group decriptors Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 11/15] ext4: add a new function which allocates bitmaps and inode tables Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 12/15] ext4: add a new function which adds a flex group to a fs Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 13/15] ext4: add new online resize interface Yongqiang Yang
2012-01-04 5:03 ` Ted Ts'o
2011-12-30 11:00 ` [PATCH V6 RESEND 14/15] ext4: let ext4_group_extend() use common code Yongqiang Yang
2011-12-30 11:00 ` [PATCH V6 RESEND 15/15] ext4: let ext4_group_add() " Yongqiang Yang
2011-12-31 17:40 ` [PATCH V6 RESEND] ext4: add new online resize Andreas Dilger
2012-01-03 20:50 ` Ted Ts'o
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=20120103210748.GE30502@thunk.org \
--to=tytso@mit.edu \
--cc=linux-ext4@vger.kernel.org \
--cc=xiaoqiangnk@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.