[RFC] Make solib_add regex-free

[Sergio Durigan Junior <sergiodj at redhat dot com>]

Hi,

While working on the lazy debuginfo reading patch, I started to dislike
the way `solib_add' handles the list of shared libraries to have their
symbols read.  It basically takes a regex as a first argument, and
matches the current so_list against it.

Well, this is not elegant IMO, especially because I believe the only
place which uses this regex feature is the `sharedlibrary' command.

Now suppose you're not the `sharedlibrary' command, and you already have
a `struct so_list *' that you want to load symbols from (happened to
me).  What you would do?  Call `solib_add' with the so->name, right?
Well, I did that, and it worked, but Jan has pointed me to a problem
that might happen:  in order to call `solib_add' correctly, you would
have to treat the so->name as a regex, i.e., escape symbols and
whatnot.  This is a pain and, IMO, error-prone.

That is why I decided to make `solib_add' take a list of `struct so_list
*' (stored in a VEC).  I believe this is much more elegant and will
probably help clarify the code.

As for the `sharedlibrary' command, I decided to make a new function
which compiles a regex, matches it against the current so_list, and
return a list containing all the shlibs which will have their symbols
loaded.  In order to get an up-to-date list of shared libs, I had to
call `update_solib_list' inside this new function, which made me break
`solib_add' in two functions to avoid calling `update_solib_list'
twice.

I tested this patch on the compile farm, without regressions.

Is this OK to check-in?

2011-08-08  Sergio Durigan Junior  <sergiodj@redhat.com>

	* solib-irix.c (irix_solib_create_inferior_hook): Uncast `solib_add'
	first argument.
	* solib-osf.c (osf_solib_create_inferior_hook): Likewise.
	* solib-sunos.c (sunos_solib_create_inferior_hook): Likewise.
	* solib.c (solib_add_1): New function which does not call
	`update_solib_list'.
	(solib_add): Split.  Call `update_solib_list' and then call
	`solib_add_1' to do the hard work.  Receive new argument `so_list'.
	Delete `pattern' argument.
	(solib_match_regex_solist): New function.
	(sharedlibrary_command): Call `solib_match_regex_solist' to obtain
	a list of shared libraries to have their symbols loaded.
	* solib.h: Include "vec.h".
	(so_list_p): New type.
	(solib_add): Update declaration; receive `VEC(so_list_p) *' as first
	argument.


diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 7bb528a..982cf51 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -495,7 +495,7 @@ irix_solib_create_inferior_hook (int from_tty)
      and will put out an annoying warning.
      Delaying the resetting of stop_soon until after symbol loading
      suppresses the warning.  */
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+  solib_add (NULL, 0, (struct target_ops *) 0, auto_solib_add);
   inf->control.stop_soon = NO_STOP_QUIETLY;
 }
 
diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index 0905eb6..0258404 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -356,7 +356,7 @@ osf_solib_create_inferior_hook (int from_tty)
      and will put out an annoying warning.
      Delaying the resetting of stop_soon until after symbol loading
      suppresses the warning.  */
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+  solib_add (NULL, 0, (struct target_ops *) 0, auto_solib_add);
   inf->control.stop_soon = NO_STOP_QUIETLY;
 }
 
diff --git a/gdb/solib-sunos.c b/gdb/solib-sunos.c
index 9936038..41c95c6 100644
--- a/gdb/solib-sunos.c
+++ b/gdb/solib-sunos.c
@@ -805,7 +805,7 @@ sunos_solib_create_inferior_hook (int from_tty)
       warning (_("shared library handler failed to disable breakpoint"));
     }
 
-  solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+  solib_add (NULL, 0, (struct target_ops *) 0, auto_solib_add);
 }
 
 static void
diff --git a/gdb/solib.c b/gdb/solib.c
index 3296ed4..5a1049d 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -888,91 +888,101 @@ libpthread_solib_p (struct so_list *so)
   return libpthread_name_p (so->so_name);
 }
 
-/* GLOBAL FUNCTION
+/* LOCAL FUNCTION
 
-   solib_add -- read in symbol info for newly added shared libraries
+   solib_add_1 -- read in symbol info for newly added shared libraries,
+		  without calling `update_solib_list'.
 
    SYNOPSIS
 
-   void solib_add (char *pattern, int from_tty, struct target_ops
-   *TARGET, int readsyms)
+   static void solib_add_1 (VEC(so_list_p) *so_list,
+			    int from_tty, int readsyms)
 
    DESCRIPTION
 
-   Read in symbolic information for any shared objects whose names
-   match PATTERN.  (If we've already read a shared object's symbol
-   info, leave it alone.)  If PATTERN is zero, read them all.
-
-   If READSYMS is 0, defer reading symbolic information until later
-   but still do any needed low level processing.
-
-   FROM_TTY and TARGET are as described for update_solib_list, above.  */
+   Helper function for `solib_add' below.  This function does all the
+   hard job described in `solib_add' comments, but does not call
+   `update_solib_list'.  */
 
-void
-solib_add (char *pattern, int from_tty,
-	   struct target_ops *target, int readsyms)
+static void
+solib_add_1 (VEC(so_list_p) *so_list, int from_tty, int readsyms)
 {
   struct so_list *gdb;
+  struct target_so_ops *ops = solib_ops (target_gdbarch);
 
   current_program_space->solib_add_generation++;
 
-  if (pattern)
-    {
-      char *re_err = re_comp (pattern);
-
-      if (re_err)
-	error (_("Invalid regexp: %s"), re_err);
-    }
-
-  update_solib_list (from_tty, target);
-
   /* Walk the list of currently loaded shared libraries, and read
      symbols for any that match the pattern --- or any whose symbols
      aren't already loaded, if no pattern was given.  */
   {
-    int any_matches = 0;
     int loaded_any_symbols = 0;
     const int flags =
         SYMFILE_DEFER_BP_RESET | (from_tty ? SYMFILE_VERBOSE : 0);
 
     for (gdb = so_list_head; gdb; gdb = gdb->next)
-      if (! pattern || re_exec (gdb->so_name))
-	{
-          /* Normally, we would read the symbols from that library
-             only if READSYMS is set.  However, we're making a small
-             exception for the pthread library, because we sometimes
-             need the library symbols to be loaded in order to provide
-             thread support (x86-linux for instance).  */
-          const int add_this_solib =
-            (readsyms || libpthread_solib_p (gdb));
-
-	  any_matches = 1;
-	  if (add_this_solib)
-	    {
-	      if (gdb->symbols_loaded)
-		{
-		  /* If no pattern was given, be quiet for shared
-		     libraries we have already loaded.  */
-		  if (pattern && (from_tty || info_verbose))
-		    printf_unfiltered (_("Symbols already loaded for %s\n"),
-				       gdb->so_name);
-		}
-	      else if (solib_read_symbols (gdb, flags))
-		loaded_any_symbols = 1;
-	    }
-	}
+      {
+	int add_this_solib;
+
+	if (so_list)
+	  {
+	    int iter, found = 0;
+	    struct so_list *cur_so;
+
+	    for (iter = 0;
+		 VEC_iterate (so_list_p, so_list, iter, cur_so);
+		 iter++)
+	      {
+		if (ops->same)
+		  {
+		    if (ops->same (gdb, cur_so))
+		      {
+			found = 1;
+			break;
+		      }
+		  }
+		else
+		  {
+		    if (!filename_cmp (gdb->so_original_name,
+				       cur_so->so_original_name))
+		      {
+			found = 1;
+			break;
+		      }
+		  }
+	      }
+
+	    if (!found)
+	      /* We are not adding this shared library's symbols.  */
+	      continue;
+	  }
+	/* Normally, we would read the symbols from that library
+	   only if READSYMS is set.  However, we're making a small
+	   exception for the pthread library, because we sometimes
+	   need the library symbols to be loaded in order to provide
+	   thread support (x86-linux for instance).  */
+	add_this_solib = (readsyms || libpthread_solib_p (gdb));
+
+	if (add_this_solib)
+	  {
+	    if (gdb->symbols_loaded)
+	      {
+		/* If no so_list was given, be quiet for shared
+		   libraries we have already loaded.  */
+		if (so_list && (from_tty || info_verbose))
+		  printf_unfiltered (_("Symbols already loaded for %s\n"),
+				     gdb->so_name);
+	      }
+	    else if (solib_read_symbols (gdb, flags))
+	      loaded_any_symbols = 1;
+	  }
+      }
 
     if (loaded_any_symbols)
       breakpoint_re_set ();
 
-    if (from_tty && pattern && ! any_matches)
-      printf_unfiltered
-	("No loaded shared libraries match the pattern `%s'.\n", pattern);
-
     if (loaded_any_symbols)
       {
-	struct target_so_ops *ops = solib_ops (target_gdbarch);
-
 	/* Getting new symbols may change our opinion about what is
 	   frameless.  */
 	reinit_frame_cache ();
@@ -982,6 +992,34 @@ solib_add (char *pattern, int from_tty,
   }
 }
 
+/* GLOBAL FUNCTION
+
+   solib_add -- read in symbol info for newly added shared libraries
+
+   SYNOPSIS
+
+   void solib_add (VEC(so_list_p) *so_list, int from_tty,
+		   struct target_ops *target, int readsyms)
+
+   DESCRIPTION
+
+   Read in symbolic information for any shared objects listed in
+   SO_LIST.  ((If we've already read a shared object's symbol info,
+   leave it alone.)  If SO_LIST is NULL, read them all.
+
+   If READSYMS is 0, defer reading symbolic information until later
+   but still do any needed low level processing inside `solib_add_1'.
+
+   FROM_TTY and TARGET are as described for update_solib_list, above.  */
+
+void
+solib_add (VEC(so_list_p) *so_list, int from_tty,
+	   struct target_ops *target, int readsyms)
+{
+  update_solib_list (from_tty, target);
+  solib_add_1 (so_list, from_tty, readsyms);
+}
+
 
 /*
 
@@ -1272,6 +1310,54 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc)
 
    LOCAL FUNCTION
 
+   solib_match_regex_solist -- compile a regex and match shared libraries
+			       against it.
+
+   SYNOPSIS
+
+   static VEC(so_list_p) *solib_match_regex_solist (const char *pattern)
+
+   DESCRIPTION
+
+   This function compiles a regex represented by PATTERN, and returns a list
+   of shared libraries matching it.  Returns NULL if PATTERN is NULL or if
+   no match was found.
+
+   This function calls `update_solib_list' in order to refresh shared the
+   shlib list.  If you are going to call `solib_add' after calling this
+   function, call `solib_add_1' instead, because this version does not
+   call `update_solib_list', thus saving time.
+
+ */
+
+static VEC(so_list_p) *
+solib_match_regex_solist (const char *pattern,
+			  struct target_ops *target, int from_tty)
+{
+  char *err;
+  struct so_list *iter;
+  VEC(so_list_p) *solist = NULL;
+
+  if (!pattern)
+    return NULL;
+
+  err = re_comp (pattern);
+
+  if (err)
+    error (_("Invalid regexp: `%s'"), err);
+
+  update_solib_list (from_tty, target);
+  for (iter = so_list_head; iter; iter = iter->next)
+    if (re_exec (iter->so_name))
+      VEC_safe_push (so_list_p, solist, iter);
+
+  return solist;
+}
+
+/*
+
+   LOCAL FUNCTION
+
    sharedlibrary_command -- handle command to explicitly add library
 
    SYNOPSIS
@@ -1285,8 +1371,19 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc)
 static void
 sharedlibrary_command (char *args, int from_tty)
 {
+  struct cleanup *c;
+  VEC(so_list_p) *solist;
+
   dont_repeat ();
-  solib_add (args, from_tty, (struct target_ops *) 0, 1);
+
+  solist = solib_match_regex_solist (args, (struct target_ops *) 0,
+				     from_tty);
+  if (!solist)
+    error (_("No shared library matched the pattern `%s'."), args);
+
+  c = make_cleanup (VEC_cleanup (so_list_p), &solist);
+  solib_add_1 (solist, from_tty, 1);
+  do_cleanups (c);
 }
 
 /* LOCAL FUNCTION
diff --git a/gdb/solib.h b/gdb/solib.h
index c473d85..f2055e7 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -21,12 +21,17 @@
 #ifndef SOLIB_H
 #define SOLIB_H
 
+#include "vec.h"
+
 /* Forward decl's for prototypes */
 struct so_list;
 struct target_ops;
 struct target_so_ops;
 struct program_space;
 
+typedef struct so_list *so_list_p;
+DEF_VEC_P(so_list_p);
+
 /* Called when we free all symtabs, to free the shared library information
    as well.  */
 
@@ -34,7 +39,7 @@ extern void clear_solib (void);
 
 /* Called to add symbols from a shared library to gdb's symbol table.  */
 
-extern void solib_add (char *, int, struct target_ops *, int);
+extern void solib_add (VEC(so_list_p) *, int, struct target_ops *, int);
 extern int solib_read_symbols (struct so_list *, int);
 
 /* Function to be called when the inferior starts up, to discover the