Bug 1027141 - Python bindings confuse signed and unsigned integers
Summary: Python bindings confuse signed and unsigned integers
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libvirt-python
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Libvirt Maintainers
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-11-06 09:15 UTC by Robie Basak
Modified: 2016-04-11 17:08 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-04-11 17:08:31 UTC


Attachments (Terms of Use)

Description Robie Basak 2013-11-06 09:15:46 UTC
The definition of py_types in python/generator.py does not use the correct PyArg_ParseTuple specifiers. It confuses signed and unsigned integers.

The C standard defines an "int" as implementation-specific signedness. You cannot rely on it being one particular way. On Intel, it's generally signed; on ARM, generally unsigned.

There exist separate PyArg_ParseTuple specifiers for unsigned integers ("i", "k" and "K" for unsigned int, unsigned long and unsigned long long respectively). When creating bindings for unsigned integer types, python/generator.py should use these specifiers to prevent potential signed/unsigned conversion errors on ARM. It looks like differentiation is already present, but the specifiers are just wrong for the unsigned cases.

I don't have a specific failure case; I noticed this when addressing a separate issue and thought I should flag this.

Comment 1 Robie Basak 2013-11-06 09:17:09 UTC
Present in commit 12dc729a711ef586ba632e90ff48667b4176f41f

Comment 2 Eric Blake 2013-11-06 13:22:06 UTC
(In reply to Robie Basak from comment #0)
> The definition of py_types in python/generator.py does not use the correct
> PyArg_ParseTuple specifiers. It confuses signed and unsigned integers.
> 
> The C standard defines an "int" as implementation-specific signedness. You
> cannot rely on it being one particular way. On Intel, it's generally signed;
> on ARM, generally unsigned.

Wrong.  'char' has implementation-specific signedness, but 'int' is always signed.  (See C99 6.2.5 paragraph 4)

> 
> There exist separate PyArg_ParseTuple specifiers for unsigned integers ("i",
> "k" and "K" for unsigned int, unsigned long and unsigned long long
> respectively). When creating bindings for unsigned integer types,
> python/generator.py should use these specifiers to prevent potential
> signed/unsigned conversion errors on ARM. It looks like differentiation is
> already present, but the specifiers are just wrong for the unsigned cases.

Using the correct specifier is still a good idea, for better overflow checking.

Comment 3 Robie Basak 2013-11-06 16:15:56 UTC
> Wrong.  'char' has implementation-specific signedness, but 'int' is always signed.  (See C99 6.2.5 paragraph 4)

Ah. Thank you for the correction.

Comment 4 Cole Robinson 2016-04-11 17:08:31 UTC
I believe this was fixed, there's a series of commits like:

commit 1bba3ca4e250c20b63515085f7be4b2c8ec5e237
Author: Pavel Hrdina <phrdina>
Date:   Tue Feb 23 13:46:45 2016 +0100

    libvirt-override: fix PyArg_ParseTuple for size_t


Note You need to log in before you can comment on or make changes to this bug.