Re: [PATCH v3 6/6] xfs: kill ialloced in xfs_dialloc()

Gao Xiang
2020-12-07T22:05:07Z

On Tue, Dec 08, 2020 at 07:23:45AM +1100, Dave Chinner wrote:
> On Mon, Dec 07, 2020 at 10:24:48PM +0800, Gao Xiang wrote:
> > On Mon, Dec 07, 2020 at 02:57:19PM +0100, Christoph Hellwig wrote:
> > > > + error = xfs_ialloc_ag_alloc(*tpp, agbp);
> > > > + if (error < 0) {
> > > > xfs_trans_brelse(*tpp, agbp);
> > > >
> > > > if (error == -ENOSPC)
> > > > error = 0;
> > > > break;
> > > > + } else if (error == 0) {
> > >
> > > No need for the else after the break.
> >
> > Personally, I'd like to save a line by using "} else if {"
> > for such case (and tell readers about these two judgments),
> > and for any cases, compilers will do their best.
>
> And extra line is not an issue, and the convention we use everywhere
> is to elide the "else" whereever possible. e.g. we do:
>
> if (foo)
> return false;
> if (!bar)
> return true;
> if (baz)
> return false;
> return true;
>
> Rather than if() {} else if() {} else if() {} else {}. The elses in
> these cases mainly obfuscate the actual logic flow...

(I mean no need to to use else if on irrelevant relationship as well)
Anyway, let me update it later...

Thanks,
Gao Xiang

>
> Cheers,
>
> Dave.
>
> --
> Dave Chinner
> david@fromorbit.com
>