From: Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org>
To: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Devicetree Compiler
<devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH v2] pylibfdt: add FdtRo.get_path()
Date: Mon, 24 Jan 2022 21:43:44 +0100 [thread overview]
Message-ID: <1902434.tdWV9SEqCh@g550jk> (raw)
In-Reply-To: <CAPnjgZ2Zj7gaT8G6WrVCKunVZgRMgef-rQLRhSRk9siR-f69DQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Simon,
On Montag, 24. Jänner 2022 18:57:47 CET Simon Glass wrote:
> Hi Luca,
>
> On Sat, 22 Jan 2022 at 03:59, Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org> 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')
>
> Do you think it would hurt to go with 4096 immediately? Python is not
> the most efficient language so it should not matter. You can also copy
> it to another object of the correct size, perhaps, to avoid memory
> wastage.
The returned string won't be of $size but of the actual string size, swig doc
mentions of the buffer size being used to dynamically allocate memory on heap
but the result will be returned as string.
Probably it's not a problem to start with a bigger size, but maybe 4096 is a
bit overkill for a normal use case? Paths (at least in Linux dts files) are
normally relatively short so 512 or 1024 feels better for this.
Which values do you think are appropriate here? I made this loop because of
David's reply to v1:
https://www.spinics.net/lists/devicetree-compiler/msg03857.html
> Also can you please explain how swig works here? I'm a bit lost as to
> the magic for calling fdt_get_path().
It's quite well described in the doc (ctrl-f for "cstring_output_maxsize")
http://www.swig.org/Doc4.0/SWIGDocumentation.html
I don't think I've worked with swig before so I found this on stackoverflow or
somewhere, but this function definitely seems to be made for this use case.
As far as I understand it, it just allocates a buffer of x bytes, does the C
call with the temporary buffer and then converts the result into a proper
Python object and discards the buffer.
Regards
Luca
next prev parent reply other threads:[~2022-01-24 20:43 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 [this message]
2022-01-25 4:48 ` David Gibson
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=1902434.tdWV9SEqCh@g550jk \
--to=luca-ifpcfpjwly+lvyrhu4qvow@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=sjg-F7+t8E8rja9g9hUCZPvPmw@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).