On 12/01/26 09:24, Peter Eisentraut wrote:
>> 0001, 0003 and 0004 looks good to me, just a small comment on 0002:
>>
>> - /*
>> - * PyModule_AddObject does not add a refcount to the object,
>> for some odd
>> - * reason; we must do that.
>> - */
>> - Py_INCREF(exc);
>> - PyModule_AddObject(mod, modname, exc);
>> -
>> /*
>> * The caller will also store a pointer to the exception
>> object in some
>> - * permanent variable, so add another ref to account for that.
>> This is
>> - * probably excessively paranoid, but let's be sure.
>> + * permanent variable, so add another ref to account for that.
>> */
>> Py_INCREF(exc);
>>
>> The later code comment say "so add another ref to account for that",
>> but
>> you've removed the previous Py_INCREF() call. The returned object
>> PyErr_NewException() already have a refcount increased for usage? If
>> that's not the case I think that the "add another ref..." don't seems
>> correct because IIUC we are increasing the ref count for the first
>> time,
>> so there is no "another" refcount being increased.
>
> The reference created by PyErr_NewException() is "stolen" by
> PyModule_AddObject(), so we need to create another one for returning
> the object from the function and storing it in the permanent
> variable. I have updated the comment in this new patch version. But
> I think the actual code is correct.
Thanks, it looks more clear IMHO now. Agree that the code is correct.
--
Matheus Alcantara
EDB: https://www.enterprisedb.com