Python integer/long conversion to C++ unsigned int/long is broken

[anderson.lizardo@…]

The following snippet in libs/python/src/converter/builtin_converters.cpp:

          return numeric_cast<T>(
              PyLong_Check(intermediate)
              ? PyLong_AsUnsignedLong(intermediate)
              : PyInt_AS_LONG(intermediate));

does not check if an exception was thrown by the Python/C conversion functions. Therefore, in case of error, these functions are returning -1, which is being cast to unsigned int.

Another missing check is when the Python integer value to be converted is negative. In this case, a proper exception (such as ValueError) must be thrown.

[anderson.lizardo@…]

Patch for bug #3189 (against boost trunk)

[anderson.lizardo@…]

Fix builtin converters for unsigned integers (v2)

[anderson.lizardo@…]

Attached a new version (v2) of the patch. Changes:

• Added more comments
• Changed exception from ValueError to OverflowError, to match Python policy on similar conversions.
• Improved test cases in libs/python/test/test_builtin_converters.py.

[anderson.lizardo@…]

Remove dependency on ctypes for tests

[anderson.lizardo@…]

Attached a new patch (boost_python_unsigned_converter_fix_no_ctypes.patch) to be applied on top of the already applied one.

This removes the dependency on ctypes added by previous patch. It seems ctypes is not available on all architectures/OSes, so instead add static methods to test_builtin_converters.cpp that return the sizes of the C++ types we need.

Also do a few renames to functions added to test_builtin_converters.py so that the line length remains sane.

[Ralf W. Grosse-Kunstleve]

Patches applied as svn revisions 54919 and 54923. Anderson, thank you very much, especially for the comprehensive tests!

[Dave Abrahams]

Replying to rwgk:

Patches applied as svn revisions 54919 and 54923. Anderson, thank you very much, especially for the comprehensive tests!

Ralf, to me this looks like an incorrect bug report. numeric_cast<T> where T is an unsigned type is supposed to throw a bad_numeric_cast if the argument is negative. Did you verify that the supplied tests fail before applying the patch?

[Dave Abrahams]

Oh, I guessthe problem is that PyLong_AsUnsignedLong returns an unsigned long, thus making the result of the ?: expression unsigned. Oops.