From: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
To: Frank Mehnert <frank.mehnert-2ptZNfhJYoRaodhZ+FW2PA@public.gmane.org>
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: libfdt / fdt_check_node_offset_ with VALID_INPUT
Date: Thu, 13 Aug 2020 17:09:45 +1000 [thread overview]
Message-ID: <20200813070945.GD17532@yekko.fritz.box> (raw)
In-Reply-To: <34725235.0rVAnNm8GT@noys4>
[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]
On Wed, Aug 12, 2020 at 05:48:56PM +0200, Frank Mehnert wrote:
> Hi all,
>
> I'm not sure if I found a bug or if I mis-use libfdt.
>
> I have a valid Linux device tree in memory and want to recursively scan
> thru it. The device tree contains a root node and several subnodes.
>
> First, I start with the root node:
>
> int root = fdt_next_node(fdt, -1, NULL);
Tangential aside: the offset of the root node is *always* 0, you don't
need to "find" it with code like this.
> Here, root is set to 0. Now I determine the offset of the first sub node:
>
> int subnode = fdt_first_subnode(fdt, root);
>
> Here, subnode is either 0 if FDT_ASSUME_MASK contains ASSUME_VALID_INPUT
> or 164 (in my case) if FDT_ASSUME_MASK does not contain ASSUME_VALID_INPUT.
That certainly sounds like a bug. Adding things to FDT_ASSUME_MASK
shouldn't change behaviour for valid inputs.
> As far as I understand, fdt_first_subnode() should not return the node
> offset of the current node if there are subnodes available.
That's correct.
> I think the problem origins at fdt_check_node_offset_() in fdt.c: If
> VALID_INPUT is set, the whole code in that function is skipped. If that
> flag is not set then fdt_next_tag(fdt, offset, &offset) is called and
> the resulting 'offset' is returned.
>
> In other words, fdt_check_node_offset_() has a side effect which depends
> on the VALID_INPUT flag.
Right. Looks like the problem is that the next if *looks* like just
an error/sanity check, which can_assume(VALID_INPUT) is bypassing.
However, it also has the fdt_next_tag() call which alters offset.
I was afraid of this sort of thing when we added the assumptions
stuff. Really we need to be running the testsuite with different
assumptions masks, but it's fiddly to do.
Care to send a patch?
[Another aside: why are you using ASSUME_VALID_INPUT - it's really
only of value if you have to run your code in an *extremely* space
limited environment, I don't recommend it as a rule]
--
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 --]
next prev parent reply other threads:[~2020-08-13 7:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-12 15:48 libfdt / fdt_check_node_offset_ with VALID_INPUT Frank Mehnert
2020-08-13 7:09 ` David Gibson [this message]
[not found] ` <20200813070945.GD17532-l+x2Y8Cxqc4e6aEkudXLsA@public.gmane.org>
2020-08-13 8:06 ` Frank Mehnert
2020-08-18 16:19 ` Rob Herring
[not found] ` <CAL_JsqLAWa-wMMiD=VffemMiqAv-3=GNehNojWrFQqED_+QDow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-08-20 10:18 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200813070945.GD17532@yekko.fritz.box \
--to=david-xt8fgy+axnrb3ne2bgzf6laj5h9x9tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=frank.mehnert-2ptZNfhJYoRaodhZ+FW2PA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).