[patch] Code cleanup-like: Do not open Python scripts twice

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

while refactoring the scripting code I have noticed that find_and_open_script
opens+fdopens a file and then Python uses the filename again for an already
opened file in PyFile_FromString instead of reusing the opened file.  It may
also introduce behavior races (OK, improbable).

Instead of building on top of this I rather cleaned it up.

There is small difference in python_run_simple_file the python file filename
will contain tildes afterwards.  But it was not an absolute filename anyway,
I do not think it matters.

Also made the code more cleanups-compliant for some unexpected throws.

No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.
I will check it in.


Thanks,
Jan


2012-01-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Do not open Python scripts twice.
	* cli/cli-cmds.c (source_script_from_stream): Pass to
	source_python_script also STREAM.
	* python/py-auto-load.c (source_section_scripts): New variable back_to,
	use it for the cleanups, call do_cleanups for it.
	(auto_load_objfile_script): Replace fclose by make_cleanup_fclose.
	* python/python-internal.h (source_python_script_for_objfile): New
	parameter file, rename parameter file to filename.
	* python/python.c (python_run_simple_file): New parameter file.
	Describe it in the function comment.  Remove variable FULL_PATH, remove
	tilde_expand call, use PyFile_FromFile intead of PyFile_FromString.
	(source_python_script, source_python_script_for_objfile)
	(source_python_script): New parameter file, rename parameter file to
	filename.  Pass FILENAME to python_run_simple_file.
	* python/python.h (source_python_script): New parameter file, rename
	parameter file to filename.

--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -531,7 +542,7 @@ source_script_from_stream (FILE *stream, const char *file)
 	{
           /* The python support reopens the file using python functions,
              so there's no point in passing STREAM here.  */
-	  source_python_script (file);
+	  source_python_script (stream, file);
 	}
       if (e.reason < 0)
 	{
--- a/gdb/python/py-auto-load.c
+++ b/gdb/python/py-auto-load.c
@@ -254,6 +254,7 @@ source_section_scripts (struct objfile *objfile, const char *source_name,
       FILE *stream;
       char *full_path;
       int opened, in_hash_table;
+      struct cleanup *back_to;
 
       if (*p != 1)
 	{
@@ -286,6 +287,13 @@ source_section_scripts (struct objfile *objfile, const char *source_name,
       opened = find_and_open_script (file, 1 /*search_path*/,
 				     &stream, &full_path);
 
+      back_to = make_cleanup (null_cleanup, NULL);
+      if (opened)
+	{
+	  make_cleanup_fclose (stream);
+	  make_cleanup (xfree, full_path);
+	}
+
       /* If one script isn't found it's not uncommon for more to not be
 	 found either.  We don't want to print an error message for each
 	 script, too much noise.  Instead, we print the warning once and tell
@@ -312,10 +320,10 @@ Use `info auto-load-scripts [REGEXP]' to list them."),
 	{
 	  /* If this file is not currently loaded, load it.  */
 	  if (! in_hash_table)
-	    source_python_script_for_objfile (objfile, full_path);
-	  fclose (stream);
-	  xfree (full_path);
+	    source_python_script_for_objfile (objfile, stream, full_path);
 	}
+
+      do_cleanups (back_to);
     }
 }
 
@@ -420,6 +428,8 @@ auto_load_objfile_script (struct objfile *objfile, const char *suffix)
     {
       struct auto_load_pspace_info *pspace_info;
 
+      make_cleanup_fclose (input);
+
       /* Add this script to the hash table too so "info auto-load-scripts"
 	 can print it.  */
       pspace_info =
@@ -431,8 +441,7 @@ auto_load_objfile_script (struct objfile *objfile, const char *suffix)
 	 It's highly unlikely that we'd ever load it twice,
 	 and these scripts are required to be idempotent under multiple
 	 loads anyway.  */
-      source_python_script_for_objfile (objfile, debugfile);
-      fclose (input);
+      source_python_script_for_objfile (objfile, input, debugfile);
     }
 
   do_cleanups (cleanups);
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -289,8 +289,8 @@ extern const struct language_defn *python_language;
 
 void gdbpy_print_stack (void);
 
-void source_python_script_for_objfile (struct objfile *objfile,
-				       const char *file);
+void source_python_script_for_objfile (struct objfile *objfile, FILE *file,
+				       const char *filename);
 
 PyObject *python_string_to_unicode (PyObject *obj);
 char *unicode_to_target_string (PyObject *unicode_str);
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -151,8 +151,8 @@ ensure_python_env (struct gdbarch *gdbarch,
   return make_cleanup (restore_python_env, env);
 }
 
-/* A wrapper around PyRun_SimpleFile.  FILENAME is the name of
-   the Python script to run.
+/* A wrapper around PyRun_SimpleFile.  FILE is the Python script to run
+   named FILENAME.
 
    One of the parameters of PyRun_SimpleFile is a FILE *.
    The problem is that type FILE is extremely system and compiler
@@ -177,25 +177,21 @@ ensure_python_env (struct gdbarch *gdbarch,
    with the Python library.  */
 
 static void
-python_run_simple_file (const char *filename)
+python_run_simple_file (FILE *file, const char *filename)
 {
-  char *full_path;
   PyObject *python_file;
   struct cleanup *cleanup;
 
-  /* Because we have a string for a filename, and are using Python to
-     open the file, we need to expand any tilde in the path first.  */
-  full_path = tilde_expand (filename);
-  cleanup = make_cleanup (xfree, full_path);
-  python_file = PyFile_FromString (full_path, "r");
+  /* Deconstification of FILENAME is a Python bug.  */
+  python_file = PyFile_FromFile (file, (char *) filename, "r",
+				 NULL /* close */);
   if (! python_file)
     {
-      do_cleanups (cleanup);
       gdbpy_print_stack ();
-      error (_("Error while opening file: %s"), full_path);
+      error (_("Error while opening file: %s"), filename);
     }
  
-  make_cleanup_py_decref (python_file);
+  cleanup = make_cleanup_py_decref (python_file);
   PyRun_SimpleFile (PyFile_AsFile (python_file), filename);
   do_cleanups (cleanup);
 }
@@ -620,17 +616,17 @@ gdbpy_parse_and_eval (PyObject *self, PyObject *args)
 }
 
 /* Read a file as Python code.
-   FILE is the name of the file.
+   FILE is the file to run.  FILENAME is name of the file FILE.
    This does not throw any errors.  If an exception occurs python will print
    the traceback and clear the error indicator.  */
 
 void
-source_python_script (const char *file)
+source_python_script (FILE *file, const char *filename)
 {
   struct cleanup *cleanup;
 
   cleanup = ensure_python_env (get_current_arch (), current_language);
-  python_run_simple_file (file);
+  python_run_simple_file (file, filename);
   do_cleanups (cleanup);
 }
 
@@ -991,19 +987,20 @@ gdbpy_progspaces (PyObject *unused1, PyObject *unused2)
    source_python_script_for_objfile; it is NULL at other times.  */
 static struct objfile *gdbpy_current_objfile;
 
-/* Set the current objfile to OBJFILE and then read FILE as Python code.
-   This does not throw any errors.  If an exception occurs python will print
-   the traceback and clear the error indicator.  */
+/* Set the current objfile to OBJFILE and then read FILE named FILENAME
+   as Python code.  This does not throw any errors.  If an exception
+   occurs python will print the traceback and clear the error indicator.  */
 
 void
-source_python_script_for_objfile (struct objfile *objfile, const char *file)
+source_python_script_for_objfile (struct objfile *objfile, FILE *file,
+                                  const char *filename)
 {
   struct cleanup *cleanups;
 
   cleanups = ensure_python_env (get_objfile_arch (objfile), current_language);
   gdbpy_current_objfile = objfile;
 
-  python_run_simple_file (file);
+  python_run_simple_file (file, filename);
 
   do_cleanups (cleanups);
   gdbpy_current_objfile = NULL;
@@ -1079,7 +1076,7 @@ eval_python_from_control_command (struct command_line *cmd)
 }
 
 void
-source_python_script (const char *file)
+source_python_script (FILE *file, const char *filename)
 {
   throw_error (UNSUPPORTED_ERROR,
 	       _("Python scripting is not supported in this copy of GDB."));
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -30,7 +30,7 @@ extern void finish_python_initialization (void);
 
 void eval_python_from_control_command (struct command_line *);
 
-void source_python_script (const char *file);
+void source_python_script (FILE *file, const char *filename);
 
 int apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 			      int embedded_offset, CORE_ADDR address,