From: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
To: Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org>
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v2] pylibfdt: add FdtRo.get_path()
Date: Tue, 25 Jan 2022 15:48:34 +1100 [thread overview]
Message-ID: <Ye+BIkvoQF7/qy9N@yekko> (raw)
In-Reply-To: <20220122105653.31219-1-luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3739 bytes --]
On Sat, Jan 22, 2022 at 11:56:54AM +0100, Luca Weiss wrote:
> Add a new Python method wrapping fdt_get_path() from the C API.
>
> Also add a test for the new method.
>
> Signed-off-by: Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org>
> ---
> Changes in v2:
> - dynamically increase size of string buffer until it fits.
> The values are chosen relatively arbitrary, and terminates at lengths
> over than 4096 as I don't expect paths to be longer and there should
> be an exit condition to not eat up RAM in case of some bug.
>
> pylibfdt/libfdt.i | 27 +++++++++++++++++++++++++++
> tests/pylibfdt_tests.py | 7 +++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> index ac70762..d103bb6 100644
> --- a/pylibfdt/libfdt.i
> +++ b/pylibfdt/libfdt.i
> @@ -443,6 +443,28 @@ class FdtRo(object):
> """
> return fdt_get_alias(self._fdt, name)
>
> + def get_path(self, nodeoffset):
> + """Get the full path of a node
> +
> + Args:
> + nodeoffset: Node offset to check
> +
> + Returns:
> + Full path to the node
> +
> + Raises:
> + FdtException if the path is longer than 'size' or other error occurs
> + """
> + size = 64
> + while size < 4096:
> + ret, path = fdt_get_path(self._fdt, nodeoffset, size)
> + if ret == -NOSPACE:
> + size += 64
> + continue
> + check_err(ret)
> + return path
> + raise ValueError('Node path is unexpectedly long')
Ah... sorry, this isn't quite what I had in mind. The idea of
resizing the buffer is to avoid having an arbitrary limit, but you
still have the 4096 byte arbitrary limit here. As Simon suggests, 4k
would probably be a good *starting* point, rather than an ending
point. Not only are repeated calls expensive in Python, but
fdt_get_path() itself is also quite expensive, since it needs to scan
the dtb from the start. I'd also suggest doubling the buffer size on
each attempt, rather than increasing linearly.
> def parent_offset(self, nodeoffset, quiet=()):
> """Get the offset of a node's parent
>
> @@ -1115,6 +1137,11 @@ typedef uint32_t fdt32_t;
> }
> }
>
> +%include "cstring.i"
> +
> +%cstring_output_maxsize(char *buf, int buflen);
> +int fdt_get_path(const void *fdt, int nodeoffset, char *buf, int buflen);
> +
> /* We have both struct fdt_property and a function fdt_property() */
> %warnfilter(302) fdt_property;
>
> diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
> index 5479363..2335a73 100644
> --- a/tests/pylibfdt_tests.py
> +++ b/tests/pylibfdt_tests.py
> @@ -348,6 +348,13 @@ class PyLibfdtBasicTests(unittest.TestCase):
> self.assertEqual("/subnode@1/subsubnode", self.fdt3.get_alias('ss1'))
> self.assertEqual("/subnode@1/subsubnode/subsubsubnode", self.fdt3.get_alias('sss1'))
>
> + def testGetPath(self):
> + """Test for the get_path() method"""
> + node = self.fdt.path_offset('/subnode@1')
> + node2 = self.fdt.path_offset('/subnode@1/subsubnode')
> + self.assertEqual("/subnode@1", self.fdt.get_path(node))
> + self.assertEqual("/subnode@1/subsubnode", self.fdt.get_path(node2))
> +
> def testParentOffset(self):
> """Test for the parent_offset() method"""
> self.assertEqual(-libfdt.NOTFOUND,
--
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:[~2022-01-25 4:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-22 10:56 [PATCH v2] pylibfdt: add FdtRo.get_path() Luca Weiss
[not found] ` <20220122105653.31219-1-luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org>
2022-01-24 17:57 ` Simon Glass
[not found] ` <CAPnjgZ2Zj7gaT8G6WrVCKunVZgRMgef-rQLRhSRk9siR-f69DQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2022-01-24 20:43 ` Luca Weiss
2022-01-25 4:48 ` David Gibson [this message]
2022-01-25 18:36 ` Luca Weiss
2022-01-26 6:35 ` 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=Ye+BIkvoQF7/qy9N@yekko \
--to=david-xt8fgy+axnrb3ne2bgzf6laj5h9x9tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=luca-IfPCFPJWly+lVyrhU4qvOw@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).