Patches to for type consistency in Python pidl
jelmer at samba.org
Sat Jun 27 16:55:53 MDT 2015
On Sat, Jun 27, 2015 at 10:52:50PM +0000, Jelmer Vernooij wrote:
> On Fri, Jun 26, 2015 at 09:53:21PM +1200, Douglas Bagnall wrote:
> > On 26/06/15 21:07, Jelmer Vernooij wrote:
> > > On Thu, Jun 25, 2015 at 06:20:33PM +1200, Douglas Bagnall wrote:
> > >> From 2b7e3afc390d69eebc8564c45bc1033c0ce6aca7 Mon Sep 17 00:00:00 2001
> > >> From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
> > >> Date: Thu, 18 Jun 2015 12:57:12 +1200
> > >> Subject: [PATCH 2/6] Treat uint32 as unsigned values in Pidl Python bindings
> > >>
> > >> This slightly increases memory use as uint32 now maps to Python Long
> > >> rather than Python Int. On the other hand the values have a better
> > >> chance of being consistent.
> > >>
> > >> Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
> > >
> > > -1
> > >
> > > This is hackish, and I don't see a good reason to do this. The memory usage
> > > is not a problem but this is actually changing the type we're returning.
> > > These are small integers, so they should just be regular int objects in Python.
> > >
> > > It's also special-casing uint32 and ignoring uint8, uint16.
> > It is special-casing uint32 because there are systems where uint32 won't fit in a
> > Python int. As currently implemented, I believe a uint32 greater than 1 << 31 will
> > appear in Python as a negative number on 32 bit systems. In Python the int and long
> > types are quite interchangeable (in Python 3, they are unified and everything
> > is a long), but the values matter.
> Ah, sure. It's common practice in Python to *only* create long objects
> for values that don't fit into a plain integer object though, rather
> than creating long objects for all values. See e.g. the behaviour of
> PyInt_FromSsize_t or PytInt_FromSize_t.
On a related note, I'm surprised we haven't seen any test failures because of
this. Do we just not have any 32 bit machines on the build farm?
More information about the samba-technical