Re: [Qemu-devel] [PATCH] sam460ex: Add comment explaining ignored errors from fdt operations

[David Gibson]

On Sat, Jul 07, 2018 at 02:23:25PM -0700, Guenter Roeck wrote:
> On Fri, Jul 06, 2018 at 02:41:18PM +0200, Paolo Bonzini wrote:
> > On 06/07/2018 03:18, David Gibson wrote:
> > >> Ok, but why would it even fail?  Maybe it's not this case, but even if
> > >> it's not fatal for the OS, generating different device trees silently
> > >> seems like a recipe for Heisenbugs.
> > > Yes, quite.  Especially since the most likely errors I can see here
> > > would actually indicate something has already gone horribly wrong with
> > > the device tree construction, so a missing clock-frequency is the
> > > least of our troubles.
> > > 
> > > I think using _FDT() here would be a better approach.
> > 
> > Or qemu_fdt_setprop_cell, which is there exactly for this reason.
> > Volunteers needed to report it in checkpatch! :)
> > 
> Sure, except it uses different parameters. Why don't you come up with
> an implementation that is acceptable to you ? I was asked earlier to add
> the comment, which I did, only to now be told that it is insufficient.
> This can go on forever. At this point, I'll be happy to send a revert
> request.

Well, we asked for the comment because we were told that ignoring the
error was intentional.  When the comment was made, it became clear
that wasn't really the case.

Really, I should have caught this when I first merged the code,
though.  The error handling throughout this function is kind of bogus,
so I'm about to add my own patch to fix it.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature