Re: [PATCH] Fix python-interactive with Python 3.6

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-01-20 11:37, Pedro Alves wrote:
> On 01/20/2017 03:15 PM, Simon Marchi wrote:
>> Since Python 3.4, the callback installed in 
>> PyOS_ReadlineFunctionPointer
>> should return a value allocated with PyMem_RawMalloc instead of
>> PyMem_Malloc.  The reason is that PyMem_Malloc must be called with the
>> Python Global Interpreter Lock (GIL) held, which is not the case in 
>> the
>> context where this function is called.  PyMem_RawMalloc was introduced
>> for cases like this.
>> 
> 
>> +/* Starting from Python 3.4, the result of the 
>> PyOS_ReadlineFunctionPointer
>> +   callback must be allocated with PyMem_RawMalloc rather than 
>> PyMem_Malloc.  */
>> +
>> +#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 4
> 
> This will break with PY_MAJOR_VERSION 4 (at some point :-) ).

Yeah, I thought about that and concluded it would be a problem for my 
grandchildren ;).

> Write:
> 
>  #if PY_MAJOR_VERSION > 3
>      || (PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 4)
> 
> ?

Ok.

> Or add some macro that makes it a bit easier to write,
> like "#if HAVE_PY_AT_LEAST(3, 4)" or
> "#if GDB_PY_VERSION >= 3004" (like GCC_VERSION).
> Or use Python's PY_VERSION_HEX, like
> "#if PY_VERSION_HEX >= 0x03040000".

I think that's better.

>> +#define PyOS_ReadlineFunctionPointer_Malloc PyMem_RawMalloc
>> +#else
>> +#define PyOS_ReadlineFunctionPointer_Malloc PyMem_Malloc
>> +#endif
> 
> It sounds like we'll find other cases that will need
> to call PyMem_RawMalloc going forward?  How about handling
> this similarly to how we handle fixing up other missing
> Python bits, in python-internal.h.  Like:

For now it looks like this is the only place where the Python API 
requires using PyMem_RawMalloc.  In the other cases, we are holding the 
GIL, thanks to gdbpy_enter, so using the regular PyMem functions is ok.  
It doesn't mean there won't be other cases added in the future though...

> #if python < 3.4
> 
> static inline void *
> gdb_PyMem_RawMalloc (size_t n)
> {
>   return gdb_PyMem_Malloc (n);
> }
> #define PyMem_RawMalloc(n) gdb_PyMem_RawMalloc (n)
> 
> #endif

... and this is cleaner anyway.  But why not just

#if python < 3.4
#define PyMem_RawMalloc PyMem_Malloc
#endif

?

>> +
>>  /* Readline function suitable for PyOS_ReadlineFunctionPointer, which
>>     is used for Python's interactive parser and raw_input.  In both
>>     cases, sys_stdin and sys_stdout are always stdin and stdout
>> @@ -63,7 +73,7 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE 
>> *sys_stdout,
>>    /* Detect EOF (Ctrl-D).  */
>>    if (p == NULL)
>>      {
>> -      q = (char *) PyMem_Malloc (1);
>> +      q = (char *) PyOS_ReadlineFunctionPointer_Malloc (1);
> 
> and then write PyMem_RawMalloc here.
> 
> Thanks,
> Pedro Alves