Broken build on OpenBSD

[Simon Glass]

Hi Alex,

On Tue, 23 Feb 2021 at 14:48, Alex G. <mr.nuke.me at gmail.com> wrote:
>
> On 2/23/21 1:07 PM, Mark Kettenis wrote:
> > Hi Simon,
> >
> > Commit c5819701a3de61e2ba2ef7ad0b616565b32305e5 broke the build on
> > OpenBSD and probably other non-Linux systems.  ENODATA, which is now
> > used in fit_check_format(), isn't defined.  It isn't part of POSIX[1]
> > and generally not available on BSD-derived systems.  Could you pick
> > another error code for this case?
>
> Hi Mark,
>
> I looked at the commit you mentioned, and I think it's fundamentally
> broken. The errors represent -EINVAL, and trying to assign different
> error codes doesn't make sense.
>
> "Wrong FIT format: no images parent node":
> -ENOENT "No such file or directory".
> This just doesn't make sense. We obviously have the file data at this
> point, and we know the data is wrong. This should be -EINVAL.
>
> "Wrong FIT format: no description":
> -ENOMSG "No message of desired type".
> Again, this doesn't make sense. We're not dealing with messaging APIs or
> send()/recv(). I think this should be -EINVAL.
>
> "Wrong FIT format: not a flattened device tree":
> -ENOEXEC "Exec format error"
> This one is amusing, as it's comparing a flattened devicetree to an
> executable. An FDT might have executable code, which is in the wrong
> format, but this is not why we're failing here.
>
> Simon,
> I'd suggest using the correct error code, which, for each case is
> -EINVAL, as the log messages also confirm: "Wrong [input value] format".
> We might have issues with the "configurations", an "@" in a signature
> name, and so forth. There just aren't enough error codes to cover the
> set of possible failures. And in any case, there likely can't be a
> reasonable 1:1 mapping to _distinct_ errno codes.
>
> Does any user even check the error code beyond "less than zero"? Take
> different decisions based on what the negative code indicates? If
> information as to what is wrong with the input value (FIT) is needed,
> then I'd suggest using a separate enum, and stick to -EINVAL.

Actually I make an effort to use different codes where possible, so
there is some indication what went wrong. Of course devs can whip out
the JTAG debugger or start filling the code with printf()s but normal
users cannot, so having an idea what is wrong is helpful.

We don't have to cover every case, but years ago U-Boot used to return
-1 for lots of failures and it was certainly frustrating to debug
things.

BTW -EINVAL is mostly reserved for of_to_plat() failure in U-Boot. It
indicates something is wrong with your devicetree data for a device.

Regards,
Simon