* [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors
@ 2018-05-04 18:37 Jonathan Marler
2018-05-14 15:56 ` Stefan Hajnoczi
2018-05-14 15:56 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Jonathan Marler @ 2018-05-04 18:37 UTC (permalink / raw)
To: crosthwaite.peter, agraf, qemu-devel; +Cc: qemu-trivial
Signed-off-by: Jonathan Marler <johnnymarler@gmail.com>
---
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);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors
2018-05-04 18:37 [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors Jonathan Marler
@ 2018-05-14 15:56 ` Stefan Hajnoczi
2018-05-14 15:56 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2018-05-14 15:56 UTC (permalink / raw)
To: Jonathan Marler
Cc: Peter Crosthwaite, Alexander Graf, qemu-devel, qemu-trivial
On Fri, May 4, 2018 at 7:37 PM, Jonathan Marler <johnnymarler@gmail.com> wrote:
> Signed-off-by: Jonathan Marler <johnnymarler@gmail.com>
> ---
> 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);
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors
2018-05-04 18:37 [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors Jonathan Marler
2018-05-14 15:56 ` Stefan Hajnoczi
@ 2018-05-14 15:56 ` Peter Maydell
2018-06-13 10:34 ` David Gibson
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2018-05-14 15:56 UTC (permalink / raw)
To: Jonathan Marler
Cc: Peter Crosthwaite, Alexander Graf, QEMU Developers, QEMU Trivial,
David Gibson
On 4 May 2018 at 19:37, Jonathan Marler <johnnymarler@gmail.com> wrote:
> Signed-off-by: Jonathan Marler <johnnymarler@gmail.com>
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.
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors
2018-05-14 15:56 ` Peter Maydell
@ 2018-06-13 10:34 ` David Gibson
0 siblings, 0 replies; 4+ messages in thread
From: David Gibson @ 2018-06-13 10:34 UTC (permalink / raw)
To: Peter Maydell
Cc: Jonathan Marler, Peter Crosthwaite, Alexander Graf,
QEMU Developers, QEMU Trivial
[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]
On Mon, May 14, 2018 at 04:56:54PM +0100, Peter Maydell wrote:
> On 4 May 2018 at 19:37, Jonathan Marler <johnnymarler@gmail.com> wrote:
> > Signed-off-by: Jonathan Marler <johnnymarler@gmail.com>
>
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-06-13 10:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 18:37 [Qemu-devel] [PATCH] handle all fdt_get_phandle_errors Jonathan Marler
2018-05-14 15:56 ` Stefan Hajnoczi
2018-05-14 15:56 ` Peter Maydell
2018-06-13 10:34 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.