Re: [PATCH v2] ubifs: compress lines for immediate return

[Heiko Schocher]

Hello Masahiro,

Am 05.09.2016 um 14:44 schrieb Masahiro Yamada:
> Hi Heiko, Richard,
>
>
>
>
> 2016-09-05 15:54 GMT+09:00 Heiko Schocher <hs@xxxxxxx>:
> > From: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> >
> > Cleanup the following code construct:
> > ret = expression;
> > if (ret)
> >          return ret;
> > return 0;
> >
> > into a simple form:
> > return expression;
> >
> > From: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> > posted on the U-Boot mailinglist.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> > Signed-off-by: Heiko Schocher <hs@xxxxxxx>
>
>
>
> I am the author of the original patch in the U-Boot ML.
>
> Please notice it has not passed the review in U-Boot ML yet.
> Actually, I got some feedback against this patch.
>
> See
> http://patchwork.ozlabs.org/patch/665199/
>
> Stephan Warren suggested that
> we should not break code uniformity.
>
>
> After I considered it and took a closer look,
> I decided that we should not do these changes.
>
>
> This patch breaks the code uniformity.
> See blow:
>
>
>
>
> >   /**
> > diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
> > index 821b348..88cd61d 100644
> > --- a/fs/ubifs/gc.c
> > +++ b/fs/ubifs/gc.c
> > @@ -297,10 +297,8 @@ static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
> >          err = dbg_check_data_nodes_order(c, &sleb->nodes);
> >          if (err)
> >                  return err;
> > -       err = dbg_check_nondata_nodes_order(c, nondata);
> > -       if (err)
> > -               return err;
> > -       return 0;
> > +
> > +       return dbg_check_nondata_nodes_order(c, nondata);
> >   }
>
> Original code has uniformity here.
>
>
> err = dbg_check_data_nodes_order(c, &sleb->nodes);
> if (err)
>         return err;
> err = dbg_check_nondata_nodes_order(c, nondata);
> if (err)
>         return err;
>
>
>
> >   /**
> > diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c
> > index ce89bdc..79a8e96 100644
> > --- a/fs/ubifs/lpt_commit.c
> > +++ b/fs/ubifs/lpt_commit.c
> > @@ -313,10 +313,7 @@ static int layout_cnodes(struct ubifs_info *c)
> >          alen = ALIGN(offs, c->min_io_size);
> >          upd_ltab(c, lnum, c->leb_size - alen, alen - offs);
> >          dbg_chk_lpt_sz(c, 4, alen - offs);
> > -       err = dbg_chk_lpt_sz(c, 3, alen);
> > -       if (err)
> > -               return err;
> > -       return 0;
> > +       return dbg_chk_lpt_sz(c, 3, alen);
>
> We have dbg_chk_lpt_sz() call just above  (its return value is ignored)
>
> So, returning the value of the last dbg_chk_lpt_sz() call
> seems a bit weird.  So, I do not want to touch this.
>
>
>
>
> Heiko,
> If you want to post this patch, it is up to you.
> But, in that case, could you drop my Author and Signed-off-by,
> then send it as your patch, please?
>
> I do not feel comfortable with my authorship
> for what I decided to not do.
>
>
> I will retract my original patch from the U-Boot ML, too.

Oh, then I was a little to fast ... sorry.

@Richard: Should we just forget this patch?

bye,
Heiko
--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany