Oh! I did miss your patch, sorry. Thanks!
It looks to me like your patch calls PyTuple_GET_ITEM() with None as the
python object (when kv is None). The python docs for PyTuple_GET_ITEM
say, "Like PyTuple_GetItem(), but does no checking of its arguments."
To me that means that the object you pass really needs to be a tuple.
Here's the patch I'm using -- it simply checks to see if kv is None,
which is true if the overload doesn't accept a keyword arg in that
position, and if so it simply rejects the overload. This is working for
me in the example and across our codebase and testsuite.
I will file a bug as you suggest, and thanks again.
Alex
--- ./boost_1_53_0/libs/python/src/object/function.cpp.orig 2013-07-22
17:38:54.000000000 -0700
+++ ./boost_1_53_0/libs/python/src/object/function.cpp 2013-08-07
10:25:26.963988000 -0700
@@ -182,6 +182,16 @@
// Get the keyword[, value pair] corresponding
PyObject* kv =
PyTuple_GET_ITEM(f->m_arg_names.ptr(), arg_pos);
+ // If kv is None, this overload does not
accept a
+ // keyword argument in this position,
meaning that
+ // the caller did not supply enough positional
+ // arguments. Reject the overload.
+ if (kv == Py_None) {
+ PyErr_Clear();
+ inner_args = handle<>();
+ break;
+ }
+
// If there were any keyword arguments,
// look up the one we need for this
// argument position
On 8/8/2013 2:28 AM, Alex Leach wrote:
Hi Alex,
On Wed, 07 Aug 2013 18:06:24 +0100, Alex Mohr <am...@pixar.com> wrote:
Thanks for your response. I've responded to your comments in-line below.
After further investigation, I believe this can be fixed by simply
checking for None when we get 'kv' from f->m_arg_names while resolving
keywords. If we encounter None, that means the overload doesn't
accept a keyword argument in that position (so not enough positional
args were passed), and we can reject the overload.
If it works, I think that's a simple, targeted fix that isn't
particularly risky. I'll try to work up a patch.
Sounds like you missed it, but I did add a fairly simple patch to my
previous email. I used your code as a test case and it worked fine; no
seg faults. I've copied it again below...
I don't have commit privileges to Boost Python, but I think from your
explanations, that this is a valid bug in BP. So it would probably be
worthwhile filing a bug report, at
http://www.boost.org/development/bugs.html
Unless someone with commit privileges wants to consider merging it? My
only thought on the patch regards return value optimisation. Would
moving the `PyObject *` declarations out of the loop impair the
compiler's ability to perform RVO?
Hope that helps anyway.
Kind regards,
Alex
--- ./boost_1_53_0/libs/python/src/object/function.cpp.orig
2013-08-07 11:02:28.569236835 +0100
+++ ./boost_1_53_0/libs/python/src/object/function.cpp 2013-08-07
11:56:10.926045853 +0100
@@ -177,16 +177,18 @@
// Grab remaining arguments by name from the
keyword dictionary
std::size_t n_actual_processed =
n_unnamed_actual;
+ PyObject *kv, *k, *value;
for (std::size_t arg_pos = n_unnamed_actual;
arg_pos < max_arity ; ++arg_pos)
{
// Get the keyword[, value pair]
corresponding
- PyObject* kv =
PyTuple_GET_ITEM(f->m_arg_names.ptr(), arg_pos);
+ kv = PyTuple_GET_ITEM(f->m_arg_names.ptr(),
arg_pos);
// If there were any keyword arguments,
// look up the one we need for this
// argument position
- PyObject* value = n_keyword_actual
- ? PyDict_GetItem(keywords,
PyTuple_GET_ITEM(kv, 0))
+ k = PyTuple_GET_ITEM(kv, 0);
+ value = (k != NULL && n_keyword_actual)
+ ? PyDict_GetItem(keywords, k)
: 0;
if (!value)
_______________________________________________
Cplusplus-sig mailing list
Cplusplus-sig@python.org
http://mail.python.org/mailman/listinfo/cplusplus-sig
