GNOME Bugzilla – Bug 602638
Accept 0 as a valid value for flag and enum arguments
Last modified: 2009-11-24 15:13:39 UTC
Several functions in Gtk+ expect that you pass 0 to mean no flags or such.
Created attachment 148275 [details] [review] Accept 0 as a valid value for flag and enum arguments
Review of attachment 148275 [details] [review]: OK for the tests (BTW, is 'flags' a reserved word in C or C++? We should remove that trailing underscore from all the flags tests). Wouldn't we prefer to pass None instead of 0 from the Python side?
Review of attachment 148275 [details] [review]: Passing None would be ambiguous in case the argument is a pointer to an enum/flag value. Let's do it with 0 as you suggested. ::: gi/pygi-argument.c @@ +390,3 @@ case GI_INFO_TYPE_FLAGS: case GI_INFO_TYPE_ENUM: + retval = 0; Not needed. @@ +391,3 @@ case GI_INFO_TYPE_ENUM: + if (PyInt_Check(object)) { + retval = 0; We should accept any object that implements the Numeric interface, not only Ints. @@ +396,3 @@ + /* Accept 0 as a valid enum and flag value */ + if (PyInt_Check(object)) { + retval = 0; Do a 'break' in the first case and drop the 'if'.
Created attachment 148308 [details] [review] Accept 0 as a valid value for flag and enum arguments
Review of attachment 148308 [details] [review]: ::: gi/pygi-argument.c @@ +388,3 @@ + PyObject *number = PyNumber_Float(object); + /* Accept 0 as a valid enum and flag value */ + if (PyNumber_Check(object)) { The marshaller doesn't support floats but only ints (which is enough, I think), so use PyNumber_Int instead. @@ +390,3 @@ + PyObject *number = PyNumber_Float(object); + /* Accept 0 as a valid enum and flag value */ + if (PyNumber_Check(object)) { retval is 1 by default. @@ +392,3 @@ + PyObject *number = PyNumber_Float(object); + /* Accept 0 as a valid enum and flag value */ + if (PyNumber_Check(object)) { If number is NULL, the exception should be cleared.
Created attachment 148380 [details] [review] Accept 0 as a valid value for flag and enum arguments
Review of attachment 148380 [details] [review]: Looks good! ::: gi/pygi-argument.c @@ +391,3 @@ + PyObject *number = PyNumber_Int(object); + /* Accept 0 as a valid enum and flag value */ + if (PyNumber_Check(object)) { Should PyErr_Clear be called when an actually exception is set? To avoid having it accidentally clear an exception we care about
Created attachment 148384 [details] [review] Accept 0 as a valid value for flag and enum arguments
Review of attachment 148384 [details] [review]: ::: tests/test_gi.py @@ +1067,3 @@ def test_flags_in(self): + TestGI.flags_in_zero(0.0) + TestGI.flags_in_zero(0) Use Number(0) (defined at the top of the test file) instead of both 0.0 and 0.
Created attachment 148386 [details] [review] Accept 0 as a valid value for flag and enum arguments
Review of attachment 148386 [details] [review]: ::: gi/pygi-argument.c @@ +391,3 @@ + PyObject *number = PyNumber_Long(object); + /* Accept 0 as a valid flag value */ + if (PyNumber_Check(object)) { Use PyNumber_Int, since _pygi_argument_from_object uses that. Ideally, both should be supported, but that's probably overkill. ::: tests/test_gi.py @@ +26,3 @@ return int(self.value) + return long(self.value) + def __long__(self): No need to add this if you address the other comment.
(In reply to comment #11) > Review of attachment 148386 [details] [review]: > > ::: gi/pygi-argument.c > @@ +391,3 @@ > + PyObject *number = PyNumber_Long(object); > + /* Accept 0 as a valid flag value */ > + if (PyNumber_Check(object)) { > > Use PyNumber_Int, since _pygi_argument_from_object uses that. > > Ideally, both should be supported, but that's probably overkill. Have gone with PyNumber_Long because the docs say PyNumber_Int is deprecated: http://docs.python.org/3.1/c-api/number.html#PyNumber_Int
That is for Python 3.1, where, iirc, longs and ints have been unified. In the meantime, they are still different types.
Created attachment 148394 [details] [review] Accept 0 as a valid value for flag and enum arguments
Review of attachment 148394 [details] [review]: Good. Thanks.
The following fix has been pushed: fc3dca0 Accept 0 as a valid value for flag and enum arguments
Created attachment 148396 [details] [review] Accept 0 as a valid value for flag and enum arguments