[PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Hui Zhu]

If the python lib dir of GDB has something wrong, each time "backtrace" will output a python error, for example:
Python Exception <type 'exceptions.ImportError'> No module named gdb:

warning:
Could not load the Python gdb module from `/usr/local/share/gdb/python'.
Limited Python support is available from the _gdb module.
Suggest passing --data-directory=/path/to/gdb/data-directory.
(gdb) start
Temporary breakpoint 1 at 0x400507: file 1.c, line 4.
Starting program: /home/teawater/tmp/1

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffe118, envp=0x7fffffffe128) at 1.c:4
4		int	a = 1;
(gdb) bt
Python Exception <type 'exceptions.ImportError'> No module named gdb.frames:
#0  main (argc=1, argv=0x7fffffffe118, envp=0x7fffffffe128) at 1.c:4

This python stack is output by function apply_frame_filter because bootstrap_python_frame_filters got NULL.
The core reason is init python dir get fail.
And if GDB doesn't init python lib in right, python script cannot use any frame filter because "import gdb" will get fail.

So if init python dir get fail, "backtrace" will print python stack.  But GDB already output error when it start.
I make a patch let it doesn't print python stack if init python dir get fail.

Please help me review it.

Thanks,
Hui

2013-11-28  Hui Zhu  <hui@codesourcery.com>

	* python/py-framefilter.c(apply_frame_filter): Add check for
	"gdb_python_module".

--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1506,7 +1506,10 @@ apply_frame_filter (struct frame_info *f
  	 initialization error.  This return code will trigger a
  	 default backtrace.  */
  
-      gdbpy_print_stack ();
+      if (gdb_python_module != NULL)
+        gdbpy_print_stack ();
+      else
+	PyErr_Clear ();
        do_cleanups (cleanups);
        return PY_BT_NO_FILTERS;
      }

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:

Hui> So if init python dir get fail, "backtrace" will print python
Hui> stack.  But GDB already output error when it start.  I make a patch
Hui> let it doesn't print python stack if init python dir get fail.

Perhaps instead the setting of gdb_python_initialized should be moved to
finish_python_initialization.

Tom

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Hui Zhu]

On 12/03/13 00:05, Tom Tromey wrote:
>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>
> Hui> So if init python dir get fail, "backtrace" will print python
> Hui> stack.  But GDB already output error when it start.  I make a patch
> Hui> let it doesn't print python stack if init python dir get fail.
>
> Perhaps instead the setting of gdb_python_initialized should be moved to
> finish_python_initialization.
>
> Tom
>

Hi Tom,

I make a new patch according to your comments.
It setup gdb_python_initialized in finish_python_initialization.

I got a issue is ensure_python_env check gdb_python_initialized and throw error if it is 0.
So gdb_python_initialized to 0 will make some commands throw error when python dir has something error.
I removed this check in the patch.

This patch pass the regression test in x86_64 linux.

Please help me review it.

Thanks,
Hui

2013-12-03  Hui Zhu  <hui@codesourcery.com>

	* python/python.c (ensure_python_env): Remove check for
	"gdb_python_initialized".
	(_initialize_python): Remove setup of "gdb_python_initialized".
	(finish_python_initialization): Add setup of
	"gdb_python_initialized".

--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -150,10 +150,6 @@ ensure_python_env (struct gdbarch *gdbar
  {
    struct python_env *env = xmalloc (sizeof *env);
  
-  /* We should not ever enter Python unless initialized.  */
-  if (!gdb_python_initialized)
-    error (_("Python not initialized"));
-
    env->state = PyGILState_Ensure ();
    env->gdbarch = python_gdbarch;
    env->language = python_language;
@@ -1717,7 +1713,6 @@ message == an error message without a st
  
    make_final_cleanup (finalize_python, NULL);
  
-  gdb_python_initialized = 1;
    return;
  
   fail:
@@ -1807,6 +1802,9 @@ finish_python_initialization (void)
       variable.  */
  
    do_cleanups (cleanup);
+
+  gdb_python_initialized = 1;
+
    return;
  
   fail:

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:

Hui> I got a issue is ensure_python_env check gdb_python_initialized and
Hui> throw error if it is 0.  So gdb_python_initialized to 0 will make
Hui> some commands throw error when python dir has something error.  I
Hui> removed this check in the patch.

I think that's a useful consistency check, so better left in place.

Two other approaches are possible here instead.  One, change
finish_python_initialization to do the needed bit of locking by handy,
not using ensure_python_env.  Or, two, don't release the GIL until
somewhere in finish_python_initialization, and then it doesn't need
to call ensure_python_env at all.

Tom

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Hui Zhu]

On 12/04/13 04:26, Tom Tromey wrote:
>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>
> Hui> I got a issue is ensure_python_env check gdb_python_initialized and
> Hui> throw error if it is 0.  So gdb_python_initialized to 0 will make
> Hui> some commands throw error when python dir has something error.  I
> Hui> removed this check in the patch.
>
> I think that's a useful consistency check, so better left in place.
>
> Two other approaches are possible here instead.  One, change
> finish_python_initialization to do the needed bit of locking by handy,
> not using ensure_python_env.  Or, two, don't release the GIL until
> somewhere in finish_python_initialization, and then it doesn't need
> to call ensure_python_env at all.
>
> Tom
>

Hi Tom,

I think the first way is better than second way because ensure_python_env has relationship with current_arch and current_language.
So I make a patch according the first way.

It extends gdb_python_initialized to a flags.  And add GDB_PYTHON_INITIALIZED_BASE and GDB_PYTHON_INITIALIZED_DIR as its flags.
In _initialize_python, set GDB_PYTHON_INITIALIZED_BASE to gdb_python_initialized.
In gdb_python_initialized, plus GDB_PYTHON_INITIALIZED_DIR to gdb_python_initialized.
Then in the function that need python lib support (apply_frame_filter, start_type_printers), check GDB_PYTHON_INITIALIZED_DIR.

It handle the issue and pass the regression test in x86_64 linux.

Please help me review it.

Thanks,
Hui

2013-12-04  Hui Zhu  <hui@codesourcery.com>

	* python/py-framefilter.c (apply_frame_filter): Change the check
	of "gdb_python_initialized".
	* python/python-internal.h (GDB_PYTHON_INITIALIZED_BASE,
	GDB_PYTHON_INITIALIZED_DIR): New.
	(gdb_python_initialized): Add comments.
	* python/python.c (gdb_python_initialized): Remove comments.
	(ensure_python_env): Change the check of "gdb_python_initialized".
	(start_type_printers): Ditto.
	(_initialize_python): Change the setup of "gdb_python_initialized".
	(finish_python_initialization): Add setup of
	"gdb_python_initialized".

--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1474,7 +1474,7 @@ apply_frame_filter (struct frame_info *f
    PyObject *item;
    htab_t levels_printed;
  
-  if (!gdb_python_initialized)
+  if ((gdb_python_initialized & GDB_PYTHON_INITIALIZED_DIR) == 0)
      return PY_BT_NO_FILTERS;
  
    cleanups = ensure_python_env (gdbarch, current_language);
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -204,6 +204,14 @@ struct program_space;
  struct bpstats;
  struct inferior;
  
+/* This bit will be true if Python libary has been successfully
+   initialized.  */
+#define GDB_PYTHON_INITIALIZED_BASE	0x1
+/* This bit will be true if Python directory has been successfully
+   initialized.  */
+#define GDB_PYTHON_INITIALIZED_DIR	0x2
+
+/* The status of Python initialized.  */
  extern int gdb_python_initialized;
  
  extern PyObject *gdb_module;
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -73,9 +73,6 @@ static const char *gdbpy_should_print_st
  #include "interps.h"
  #include "event-top.h"
  
-/* True if Python has been successfully initialized, false
-   otherwise.  */
-
  int gdb_python_initialized;
  
  static PyMethodDef GdbMethods[];
@@ -151,7 +148,7 @@ ensure_python_env (struct gdbarch *gdbar
    struct python_env *env = xmalloc (sizeof *env);
  
    /* We should not ever enter Python unless initialized.  */
-  if (!gdb_python_initialized)
+  if ((gdb_python_initialized & GDB_PYTHON_INITIALIZED_BASE) == 0)
      error (_("Python not initialized"));
  
    env->state = PyGILState_Ensure ();
@@ -1235,7 +1232,7 @@ start_type_printers (void)
    struct cleanup *cleanups;
    PyObject *type_module, *func = NULL, *result_obj = NULL;
  
-  if (!gdb_python_initialized)
+  if ((gdb_python_initialized & GDB_PYTHON_INITIALIZED_DIR) == 0)
      return NULL;
  
    cleanups = ensure_python_env (get_current_arch (), current_language);
@@ -1717,7 +1714,7 @@ message == an error message without a st
  
    make_final_cleanup (finalize_python, NULL);
  
-  gdb_python_initialized = 1;
+  gdb_python_initialized = GDB_PYTHON_INITIALIZED_BASE;
    return;
  
   fail:
@@ -1807,6 +1804,8 @@ finish_python_initialization (void)
       variable.  */
  
    do_cleanups (cleanup);
+
+  gdb_python_initialized |= GDB_PYTHON_INITIALIZED_DIR;
    return;
  
   fail:

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:

>> Two other approaches are possible here instead.  One, change
>> finish_python_initialization to do the needed bit of locking by handy,
>> not using ensure_python_env.  Or, two, don't release the GIL until
>> somewhere in finish_python_initialization, and then it doesn't need
>> to call ensure_python_env at all.

Hui> I think the first way is better than second way because
Hui> ensure_python_env has relationship with current_arch and
Hui> current_language.  So I make a patch according the first way.

After seeing the patch I think it is probably preferable to do the
second route -- move the GIL releasing to finish_python_initialization.

Can you try the appended?

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 35a1d73..6554be2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2013-12-11  Tom Tromey  <tromey@redhat.com>
+
+	* python/python.c (_initialize_python): Don't release the GIL or
+	set gdb_python_initialized.
+	(release_gil): New function.
+	(finish_python_initialization): Use release_gil.  Don't call
+	ensure_python_env.  Set gdb_python_initialized.
+
 2013-12-11  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	* break-catch-throw.c (fetch_probe_arguments): Pass selected frame
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1873936..ddf8a1a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1711,18 +1711,12 @@ message == an error message without a stack will be printed."),
   if (gdbpy_value_cst == NULL)
     goto fail;
 
-  /* Release the GIL while gdb runs.  */
-  PyThreadState_Swap (NULL);
-  PyEval_ReleaseLock ();
-
   make_final_cleanup (finalize_python, NULL);
 
-  gdb_python_initialized = 1;
   return;
 
  fail:
   gdbpy_print_stack ();
-  /* Do not set 'gdb_python_initialized'.  */
   return;
 
 #endif /* HAVE_PYTHON */
@@ -1730,6 +1724,15 @@ message == an error message without a stack will be printed."),
 
 #ifdef HAVE_PYTHON
 
+/* A cleanup function that releases the GIL.  */
+
+static void
+release_gil (void *ignore)
+{
+  PyThreadState_Swap (NULL);
+  PyEval_ReleaseLock ();
+}
+
 /* Perform the remaining python initializations.
    These must be done after GDB is at least mostly initialized.
    E.g., The "info pretty-printer" command needs the "info" prefix
@@ -1743,7 +1746,8 @@ finish_python_initialization (void)
   PyObject *sys_path;
   struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  /* Release the GIL while gdb runs.  */
+  cleanup = make_cleanup (release_gil, NULL);
 
   /* Add the initial data-directory to sys.path.  */
 
@@ -1807,12 +1811,16 @@ finish_python_initialization (void)
      variable.  */
 
   do_cleanups (cleanup);
+
+  gdb_python_initialized = 1;
+
   return;
 
  fail:
   gdbpy_print_stack ();
   warning (_("internal error: Unhandled Python exception"));
   do_cleanups (cleanup);
+  /* Do not set 'gdb_python_initialized'.  */
 }
 
 #endif /* HAVE_PYTHON */

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Hui Zhu]

On 12/12/13 00:30, Tom Tromey wrote:
>>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:
>
>>> Two other approaches are possible here instead.  One, change
>>> finish_python_initialization to do the needed bit of locking by handy,
>>> not using ensure_python_env.  Or, two, don't release the GIL until
>>> somewhere in finish_python_initialization, and then it doesn't need
>>> to call ensure_python_env at all.
>
> Hui> I think the first way is better than second way because
> Hui> ensure_python_env has relationship with current_arch and
> Hui> current_language.  So I make a patch according the first way.
>
> After seeing the patch I think it is probably preferable to do the
> second route -- move the GIL releasing to finish_python_initialization.
>
> Can you try the appended?

Hi Tom,

With this patch, GDB will output a lot of "Python not initialized" that output by function "ensure_python_env".
For example:
(gdb) start
Temporary breakpoint 1 at 0x400507: file 2.c, line 4.
Starting program: /home/teawater/tmp/2
Python not initialized
(gdb) bt
#0  main (argc=1, argv=0x7fffffffe138, envp=0x7fffffffe148) at 2.c:4
(gdb) quit
A debugging session is active.

	Inferior 1 [process 31052] will be killed.

Quit anyway? (y or n) y
Python not initialized

Thanks,
Hui

>
> Tom
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 35a1d73..6554be2 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2013-12-11  Tom Tromey  <tromey@redhat.com>
> +
> +	* python/python.c (_initialize_python): Don't release the GIL or
> +	set gdb_python_initialized.
> +	(release_gil): New function.
> +	(finish_python_initialization): Use release_gil.  Don't call
> +	ensure_python_env.  Set gdb_python_initialized.
> +
>   2013-12-11  Sergio Durigan Junior  <sergiodj@redhat.com>
>
>   	* break-catch-throw.c (fetch_probe_arguments): Pass selected frame
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 1873936..ddf8a1a 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1711,18 +1711,12 @@ message == an error message without a stack will be printed."),
>     if (gdbpy_value_cst == NULL)
>       goto fail;
>
> -  /* Release the GIL while gdb runs.  */
> -  PyThreadState_Swap (NULL);
> -  PyEval_ReleaseLock ();
> -
>     make_final_cleanup (finalize_python, NULL);
>
> -  gdb_python_initialized = 1;
>     return;
>
>    fail:
>     gdbpy_print_stack ();
> -  /* Do not set 'gdb_python_initialized'.  */
>     return;
>
>   #endif /* HAVE_PYTHON */
> @@ -1730,6 +1724,15 @@ message == an error message without a stack will be printed."),
>
>   #ifdef HAVE_PYTHON
>
> +/* A cleanup function that releases the GIL.  */
> +
> +static void
> +release_gil (void *ignore)
> +{
> +  PyThreadState_Swap (NULL);
> +  PyEval_ReleaseLock ();
> +}
> +
>   /* Perform the remaining python initializations.
>      These must be done after GDB is at least mostly initialized.
>      E.g., The "info pretty-printer" command needs the "info" prefix
> @@ -1743,7 +1746,8 @@ finish_python_initialization (void)
>     PyObject *sys_path;
>     struct cleanup *cleanup;
>
> -  cleanup = ensure_python_env (get_current_arch (), current_language);
> +  /* Release the GIL while gdb runs.  */
> +  cleanup = make_cleanup (release_gil, NULL);
>
>     /* Add the initial data-directory to sys.path.  */
>
> @@ -1807,12 +1811,16 @@ finish_python_initialization (void)
>        variable.  */
>
>     do_cleanups (cleanup);
> +
> +  gdb_python_initialized = 1;
> +
>     return;
>
>    fail:
>     gdbpy_print_stack ();
>     warning (_("internal error: Unhandled Python exception"));
>     do_cleanups (cleanup);
> +  /* Do not set 'gdb_python_initialized'.  */
>   }
>
>   #endif /* HAVE_PYTHON */
>

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey]

>>>>> "Hui" == Hui Zhu <hui_zhu@mentor.com> writes:

Hui> With this patch, GDB will output a lot of "Python not initialized"
Hui> that output by function "ensure_python_env".
Hui> For example:
Hui> (gdb) start
Hui> Temporary breakpoint 1 at 0x400507: file 2.c, line 4.
Hui> Starting program: /home/teawater/tmp/2
Hui> Python not initialized

I think this may be an independent bug.
I'll have a better patch soon.

Tom

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey]

>>>>> "Hui" == Tom Tromey <tromey@redhat.com> writes:

Hui> With this patch, GDB will output a lot of "Python not initialized"
Hui> that output by function "ensure_python_env".
Hui> For example:
Hui> (gdb) start
Hui> Temporary breakpoint 1 at 0x400507: file 2.c, line 4.
Hui> Starting program: /home/teawater/tmp/2
Hui> Python not initialized

Tom> I think this may be an independent bug.
Tom> I'll have a better patch soon.

Ok, I have looked into the problem some more.
I'm sorry this has been taking so long.

I've appended my current patch.  It arranges to clear
gdb_python_initialized if the second stage of initialization fails.
And, it adds a check on gdb_python_initialized in a couple of spots that
we missed before.

However, it still isn't quite right, due to this code in
finish_python_initialization:

  if (gdb_python_module == NULL)
    {
      gdbpy_print_stack ();
      /* This is passed in one call to warning so that blank lines aren't
	 inserted between each line of text.  */
      warning (_("\n"
		 "Could not load the Python gdb module from `%s'.\n"
		 "Limited Python support is available from the _gdb module.\n"
		 "Suggest passing --data-directory=/path/to/gdb/data-directory.\n"),
		 gdb_pythondir);
      do_cleanups (cleanup);
      return;
    }

The question is -- what should we really do here?

One option is to set gdb_python_initialized = 0.
I think this makes the warning text obsolete, though, since the
"limited" support would not actually be available at all (e.g., the
"python" command wouldn't work at all).

Another option is to try to split the initialization global.  One of
your patches in this thread did that.  It wasn't clear to me that your
patch went far enough, though -- it changed some spots to check the new
flag, but not every spot.  This approach would seem to involve auditing
all uses of gdb_python_initialized and deciding (how?) whether or not
the particular spot needs "full" initialization.

I lean toward the simpler solution of changing the warning message and
clearing the flag.

Tom

2014-01-14  Tom Tromey  <tromey@redhat.com>

	* python/py-finishbreakpoint.c (bpfinishpy_handle_stop): Check
	gdb_python_initialized.
	(bpfinishpy_handle_exit): Likewise.
	* python/python.c (_initialize_python): Don't release the GIL.
	(release_gil): New function.
	(finish_python_initialization): Use release_gil.  Don't call
	ensure_python_env.  Possibly clear gdb_python_initialized.

diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 712a9ee..4e052d7 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -382,8 +382,12 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b, void *args)
 static void
 bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
 {
-  struct cleanup *cleanup = ensure_python_env (get_current_arch (),
-                                               current_language);
+  struct cleanup *cleanup;
+
+  if (!gdb_python_initialized)
+    return;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
 
   iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb,
                             bs == NULL ? NULL : bs->breakpoint_at);
@@ -397,8 +401,12 @@ bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
 static void
 bpfinishpy_handle_exit (struct inferior *inf)
 {
-  struct cleanup *cleanup = ensure_python_env (target_gdbarch (),
-                                               current_language);
+  struct cleanup *cleanup;
+
+  if (!gdb_python_initialized)
+    return;
+
+  cleanup = ensure_python_env (target_gdbarch (), current_language);
 
   iterate_over_breakpoints (bpfinishpy_detect_out_scope_cb, NULL);
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 337c170..8a6389f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1712,10 +1712,6 @@ message == an error message without a stack will be printed."),
   if (gdbpy_value_cst == NULL)
     goto fail;
 
-  /* Release the GIL while gdb runs.  */
-  PyThreadState_Swap (NULL);
-  PyEval_ReleaseLock ();
-
   make_final_cleanup (finalize_python, NULL);
 
   gdb_python_initialized = 1;
@@ -1731,6 +1727,15 @@ message == an error message without a stack will be printed."),
 
 #ifdef HAVE_PYTHON
 
+/* A cleanup function that releases the GIL.  */
+
+static void
+release_gil (void *ignore)
+{
+  PyThreadState_Swap (NULL);
+  PyEval_ReleaseLock ();
+}
+
 /* Perform the remaining python initializations.
    These must be done after GDB is at least mostly initialized.
    E.g., The "info pretty-printer" command needs the "info" prefix
@@ -1744,7 +1749,13 @@ finish_python_initialization (void)
   PyObject *sys_path;
   struct cleanup *cleanup;
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  /* If the first phase of initialization failed, don't bother doing
+     anything here.  */
+  if (!gdb_python_initialized)
+    return;
+
+  /* Release the GIL while gdb runs.  */
+  cleanup = make_cleanup (release_gil, NULL);
 
   /* Add the initial data-directory to sys.path.  */
 
@@ -1814,6 +1825,7 @@ finish_python_initialization (void)
   gdbpy_print_stack ();
   warning (_("internal error: Unhandled Python exception"));
   do_cleanups (cleanup);
+  gdb_python_initialized = 0;
 }
 
 #endif /* HAVE_PYTHON */

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Phil Muldoon]

On Tue, 14 Jan 2014, Tom Tromey wrote:

> >>>>> "Hui" == Tom Tromey <tromey@redhat.com> writes:
> 
> I've appended my current patch.  It arranges to clear
> gdb_python_initialized if the second stage of initialization fails.
> And, it adds a check on gdb_python_initialized in a couple of spots that
> we missed before.
> 
> However, it still isn't quite right, due to this code in
> finish_python_initialization:
> 
>   if (gdb_python_module == NULL)
>     {
>       gdbpy_print_stack ();
>       /* This is passed in one call to warning so that blank lines aren't
> 	 inserted between each line of text.  */
>       warning (_("\n"
> 		 "Could not load the Python gdb module from `%s'.\n"
> 		 "Limited Python support is available from the _gdb module.\n"
> 		 "Suggest passing --data-directory=/path/to/gdb/data-directory.\n"),
> 		 gdb_pythondir);
>       do_cleanups (cleanup);
>       return;
>     }
> 
> The question is -- what should we really do here?
> 
> One option is to set gdb_python_initialized = 0.
> I think this makes the warning text obsolete, though, since the
> "limited" support would not actually be available at all (e.g., the
> "python" command wouldn't work at all).

I think the suggestion in the warning text is still good though?

> I lean toward the simpler solution of changing the warning message and
> clearing the flag.

I think so too, it makes things simpler for the user.  But in this
patch you deleted the warning, so not sure if the patch you posted is
your preferred solution?

Cheers,

Phil

Re: [PATCH] Make "backtrace" doesn't print python stack if init python dir get fail

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> I think the suggestion in the warning text is still good though?

Yeah, good point.

>> I lean toward the simpler solution of changing the warning message and
>> clearing the flag.

Phil> I think so too, it makes things simpler for the user.  But in this
Phil> patch you deleted the warning, so not sure if the patch you posted is
Phil> your preferred solution?

No, the patch left the message there.  I looked again just now to make
sure :)

Tom