[U-Boot] [PATCH 2/8] dm: core: remove meaningless if conditional

[Masahiro Yamada <yamada.m@jp.panasonic.com>]

Hi Simon,



On Mon, 17 Nov 2014 09:14:15 +0000
Simon Glass <sjg@chromium.org> wrote:

> Hi Masahiro,
> 
> On 17 November 2014 08:19, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> > If the variable "ret" is equal to "-ENOENT", it is trapped at [1] and
> > never reaches [2].  At [3], the condition "ret != -ENOENT" is always
> > true.
> >
> >   if (ret == -ENOENT) {                       <------------------ [1]
> >           continue;
> >   } else if (ret == -ENODEV) {
> >           dm_dbg("Device '%s' has no compatible string\n", name);
> >           break;
> >   } else if (ret) {                           <------------------ [2]
> >           dm_warn("Device tree error at offset %d\n", offset);
> >           if (!result || ret != -ENOENT)      <------------------ [3]
> >                   result = ret;
> >           break;
> >   }
> >
> > Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> > ---
> >
> >  drivers/core/lists.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/core/lists.c b/drivers/core/lists.c
> > index 3a1ea85..0aad56d 100644
> > --- a/drivers/core/lists.c
> > +++ b/drivers/core/lists.c
> > @@ -136,8 +136,7 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
> >                         break;
> >                 } else if (ret) {
> >                         dm_warn("Device tree error at offset %d\n", offset);
> > -                       if (!result || ret != -ENOENT)
> > -                               result = ret;
> > +                       result = ret;
> 
> Thanks for the clear explanation.
> It looks good, except my intent was to return the first error, hence
> the 'if (!result ...'.
> 
> Your code will return the last error. Can we preserve the current bahaviour?


Do you mean, like this?

           dm_warn("Device tree error at offset %d\n", offset);
           if (!result)
                   result = ret;
           break;


I think there is no difference in the behaviour, because
this "for" loop is escaped out at the first encounter with an error
except -ENOENT.

Here is the only line to set "result" and it is followed by "break" statement,
so the last error is also the first error, I think.



Best Regards
Masahiro Yamada