On Mon, May 14, 2018 at 04:56:54PM +0100, Peter Maydell wrote: > On 4 May 2018 at 19:37, Jonathan Marler <[email protected]> wrote: > > Signed-off-by: Jonathan Marler <[email protected]> > > Hi; thanks for this patch. > > > --- > > device_tree.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/device_tree.c b/device_tree.c > > index 52c3358..2b75905 100644 > > --- a/device_tree.c > > +++ b/device_tree.c > > @@ -379,8 +379,12 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char > > *path) > > > > r = fdt_get_phandle(fdt, findnode_nofail(fdt, path)); > > if (r == 0) { > > - error_report("%s: Couldn't get phandle for %s: %s", __func__, > > - path, fdt_strerror(r)); > > + error_report("%s: Node %s does not have a 'phandle'", __func__, > > + path); > > + exit(1); > > + } > > + if (r == -1) { > > + error_report("%s: Couldn't get phandle for %s", __func__, path); > > exit(1); > > } > > Could you explain in what situation this is needed? The documentation > for fdt_get_phandle() says > * returns: > * the phandle of the node at nodeoffset, on success (!= 0, != -1) > * 0, if the node has no phandle, or another error occurs > > which I interpret to mean that it is not possible for it to return -1.
I *think* that was the intention; it's a long time since I wrote it,
so I'm not sure. However, looking at the implementation, that's not
strictly the case.
We certainly return 0 for any error we explicitly detect. However, if
the node has a "phandle" property that's correctly formed but contains
-1, then we'll return -1. Of course, a correct dt blob won't have
such a property, but then we do detect a bunch of other conditions
that a correct dt shouldn't have.
On balance, I'm thinking I should probably change libfdt to check for
a -1 phandle property and return 0 for that case, like all other error
cases.
--
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
