Re: [PATCH v1 3/4] gdb/python: migrate Python initialization to use the new config API (PEP 741)

[Matthieu Longo <matthieu.longo@arm.com>]

On 17/04/2026 16:53, Tom Tromey wrote:
>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
> 
> Matthieu> +  PyInitConfig *m_config;
> Matthieu> +  bool m_err;
> Matthieu> +  bool m_py_initialized;
> 
> These last two can just say '= false'
> 

Fixed.

> Matthieu> +};
> Matthieu> +
> Matthieu> +gdb_PyInitializer::gdb_PyInitializer ()
> Matthieu> +  : m_config (PyInitConfig_Create ()),
> Matthieu> +    m_err (false),
> Matthieu> +    m_py_initialized (false)
> 
> ... then they don't have to be initialized here.
> 
> Matthieu> +{
> Matthieu> +  if (m_config == nullptr)
> Matthieu> +  {
> Matthieu> +    throw_error (GENERIC_ERROR,
> Matthieu> +		 _("Python initialization failed: memory allocation failed"));
> 
> No need for the braces.  Also this can just be error (...), throw_error
> is normally only for special things.
> 

How is it supposed to work with error() ?
error() calls exit(EXIT_FAILURE).

It seems to me that throw_error is needed here, otherwise the code in ...

> Matthieu> +  const char *err_msg;
> Matthieu> +  if (PyInitConfig_GetError (m_config, &err_msg) == 1)
> Matthieu> +    throw_error (GENERIC_ERROR,
> Matthieu> +		 _("Python initialization failed: %s"),
> Matthieu> +		 err_msg);
> Matthieu> +
> Matthieu> +  int exit_code;
> Matthieu> +  gdb_assert (PyInitConfig_GetExitCode (m_config, &exit_code) != 0);
> 
> Can't combine a side effect with an assert here.
> 

Fixed.

> Matthieu> +  throw_error (GENERIC_ERROR,
> Matthieu> +	       _("Python initialization failed with exit status: %d"),
> Matthieu> +	       exit_code);
> 
> Also same here about error; and does this code need to clear the Python
> error or anything like that?
> 

In my understanding, the error should not be cleared.
The doc (https://docs.python.org/3/c-api/init_config.html#c.PyInitConfig_GetError) says:
   The error message remains valid until another PyInitConfig function is
   called with config. The caller doesn’t have to free the error message.

> Matthieu> +  catch (const gdb_exception_error &exc)
> Matthieu> +  {
> Matthieu> +    gdb_printf ("%s\n", exc.what ());
> 
> Use exception_print instead, probably with gdb_stderr.
> 

... here is irrelevant.

Am I misunderstanding something ?

> thanks,
> Tom

Matthieu