Re: [PATCH v4] pylibfdt: add FdtRo.get_path()

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2630 bytes --]

On Wed, Apr 20, 2022 at 03:29:49PM -0600, Simon Glass wrote:
> Hi Luca,
> 
> On Wed, 20 Apr 2022 at 12:20, Luca Weiss <luca-IfPCFPJWly+lVyrhU4qvOw@public.gmane.org> wrote:
> >
> > Hi Simon and David,
> >
> > On Mittwoch, 20. April 2022 04:56:15 CEST David Gibson wrote:
> > > On Tue, Apr 19, 2022 at 03:54:07PM -0600, Simon Glass wrote:
> > > > Hi Luca,
> > > >
> > > > On Tue, 19 Apr 2022 at 13:49, 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 since v3:
> > > > > * add quiet parameter
> > > > > * return integer error when error is quieted
> > > > >
> > > > > Changes since v2:
> > > > > * Remove arbitrary size limit
> > > > > * Change size calculation to increase exponentially
> > > > > * Add test to verify we still get exceptions with bad parameters
> > > > >
> > > > >  pylibfdt/libfdt.i       | 28 ++++++++++++++++++++++++++++
> > > > >  tests/pylibfdt_tests.py | 13 +++++++++++++
> > > > >  2 files changed, 41 insertions(+)
> > > >
> > > > Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > >
> > > > You don't check for when it is output of space, but I think that is
> > > > OK.
> > >
> > > Right, a testcase for that would be good, but it can be a follow up.
> >
> > I don't quite understand what you mean "is output of space".
> 
> is out of space (sorry)
> 
> >
> > Maybe you mean a test for when the string is longer than 1024 characters
> > ("size = 1024") so the size increase gets triggered?
> >
> > If yes, then I did test that manually before by setting the value absurdly
> > low. Not sure how to properly check that in the tests though. Add a node with
> > a super long name into the test dtb?
> 
> Another option is to add a parameter for the max size, just for
> testing, but I doubt David would like that!

Actually I'm fine with adding a "hint" size as a parameter - as long
as it's optional with a a sensible default.

> You could have a SIZE_INCREMENT = 1024 at the top of the file, then
> change it in a try/finally clause in your test.
> 
> >
> > >
> > > Applied.
> >
> > Thanks!
> >
> > Regards
> > Luca
> 
> Regards,
> Simon
> 

-- 
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 --]