Hi Masahiro, On 17 November 2014 11:19, Masahiro Yamada <[email protected]> wrote: > Hi Simon, > > > > On Mon, 17 Nov 2014 09:14:15 +0000 > Simon Glass <[email protected]> wrote: > >> Hi Masahiro, >> >> On 17 November 2014 08:19, Masahiro Yamada <[email protected]> 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 <[email protected]> >> > --- >> > >> > 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.
Ah yes, OK. The intent here is to provide a useful error message without adding a lot of noise. It's a bit of a pain but we should keep it I think. Your patch looks good. Acked-by: Simon Glass <[email protected]> Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
