On Sat, Jun 09, 2018 at 11:12:44AM -0800, Simon Glass wrote:
> Hi David,
>
> On 8 June 2018 at 04:27, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Wed, Jun 06, 2018 at 03:37:02PM -0600, Simon Glass wrote:
> >> Export all of these through Python.
> >>
> >> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
> >> ---
> >>
> >> Changes in v2:
> >> - Drop use of check_err() since these functions cannot fail
> >> - Update existing header functions to also drop check_err()
> >>
> >> pylibfdt/libfdt.i | 71 +++++++++++++++++++++++++++++++++++++++--
> >> tests/pylibfdt_tests.py | 8 +++++
> >> 2 files changed, 76 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
> >> index 94c3d00..f33e2ab 100644
> >> --- a/pylibfdt/libfdt.i
> >> +++ b/pylibfdt/libfdt.i
> >> @@ -252,21 +252,86 @@ class Fdt:
> >> """
> >> return check_err(fdt_next_subnode(self._fdt, nodeoffset), quiet)
> >>
> >> + def magic(self):
> >> + """Return the magic word from the header
> >> +
> >> + Returns:
> >> + Magic word
> >> + """
> >> + # Use a mask to ensure that this does not return a -ve number
> >> + return fdt_magic(self._fdt) & 0xffffffff
> >
> > You still have the mask here for no clear reason.
>
> Python's numbers are a little strange. If the top bit of a 32-bit
> number is set, this means it is a negative number in two-complement
> arithmetic, as you know. Python will then use a negative value instead
> of positive. To force it to regard the number as unsigned, we must
> mask it. This is how I have learned to do it in Python, but maybe
> there is a better way?
Two observations first:
1) I'm pretty sure this will only apply on 32-bit platforms
(specifically those where a C "long" and therefore a Python int() are
32-bit).
2) I think this has more to do with the swig typemapping that the
integers in Python per se. On a 32-bit platform I can do this in
Python:
>>> (1 << 31)
2147483648L
So we're getting the expected answer... but as a Python long rather
than a Python int (it's a Python int when run on a 64-bit platform).
So what seems to be happening here is that on the C side (not sure if
it's in stuff you've written or in swig's generated magic), we're
forcing the 'uint32_t' into a 'long', then stuffing that into a Python
integer.
The mask strips away the sign, and at the same time coerces the value
into a Python long (because 0xffffffff will be a PyLong on such a
platform).
> To see the impact, try removing the mask. You will see that magic()
> will return a -ve number, rather than unsigned.
So, the thing that bothers me a bit here is that it's not the same for
all the header functions, even though they all return uint32_t on the
C side. I guess we get away with in practice, because apart from
'magic' all the other values should actually fit in 31 bits. Could
still cause confusing answers on a malformed fdt, of course.
I think the right solution is to alter the swig stuff so that on
platforms where sizeof(long) <= 4 we make the header functions return
a PyLong instead of a PyInt. I'm not immediately sure how to do that,
however.
I guess the current approach will work for now, even if it is a hack,
so I'll go ahead and apply it, and we can try to fix it up later.
--
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:
signature.asc
Description: PGP signature
