[caiman-discuss] code review for 3406

[Evan Layton]

Ethan Quach wrote:
> 
> 
> Evan Layton wrote:
>> Hi Ethan,
>>
>> I'm still not the best at python but from what little knowledge I have 
>> these changes look pretty good. I do like that you've moved all the 
>> Py_buildValue calls to one place so we don't have that line repeated 
>> in several places.
>>
>> One question on this: In some of the earlier discussions on this it 
>> was stated the the line "return (Py_BuildValue("[iO]", ret, 
>> listOfDicts));" "[is]" should be used instead of "[iO]" since it's 
>> possible to return a NULL from be_list() when there are no BE's on a 
>> system. Why did we stick with "[iO]"?
> 
> We were talking about using [is] instead of [iO] only for error cases:
> 
>    if error
>            Py_BuildValue("[is]" ...., ...)
>    else
>            Py_BuildValue("[iO]", ..., ...)
> 
> but it seemed pretty hacky to swith to 's' (string) from 'O' (object) just
> because Py_BuildValue() doesn't handle NULL for 'O', and out item is
> NULL.
> 
> 
> As long as listOfDicts is never NULL, (and it shouldn't be because
> we're always initializing it at 288), using [iO] is more proper.

Ah okay that makes sense, Thanks for the explanation! :-)

-evan

> 
> 
> -ethan
> 
>>
>> Thanks!
>> -evan
>>
>> Ethan Quach wrote:
>>> Need to get this reviewed asap.  This needs to go into the next build.
>>>
>>>
>>> Webrev
>>> http://cr.opensolaris.org/~equach/webrev.3406
>>>
>>>
>>> Defect
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3406
>>>
>>>
>>>
>>> thanks,
>>> -ethan
>>>
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>