Re: Resubmital of: shared libraries and a remote target

[Elena Zannoni <ezannoni at cygnus dot com>]

Stephen Smith writes:
 > This patch supersedes one component of  http://sources.redhat.com/ml/gdb-patches/2001-07/msg00448.html
 > as requested by Elena Zannoni <ezannoni@cygnus.com>. I ran the remote.c code through
 > indent 2.2.6 asking to to limit line lengths to 80 chars as requested by
 > Kevin Buettner <kevinb@cygnus.com>
 > 

Looks much better thanks.

 > I didn't change the call to symbol_file_add instead of add_symbol_file_command since that
 > would mean inserting duplicate code into gdb to do the parsing of the input string.
 > 

Actually, I assume you added the qLibraries packet, so you can control
the format of the response, right? Can it be changed?  The command
line functions (*_command) should be used only from the CLI.  The
grand plan is to separate all the CLI code and put it into the cli
subdirectory, where it won't be accessible from other parts of gdb.
So, it would be better if you could manipulate the response to be
better suited for the symbol_file_add command.  Actually you can see
how that function is invoked from the shared libraries files, and
maybe do something similar as well.

I still wonder about the need for startup options, though. The code
would be simpler if there was a command to enable/disable this
feature.  Since this feature works only for remote targets, it
wouldn't make sense if one is running GDB natively. I think of startup
options as something that is always going to work.

Elena


 > sps
 > 
 >         * main.c (--remote-shared-libs, --no-remote-shared-libs): New
 >         switches.
 >         * remote.c (remote_get_list_of_shared_libraries, findFile): New
 >         functions which support an option extention to the remote protocol.
 >         * symfile.c (add_symbol_file_command): Make extern.
 >         * symfile.h (add_symbol_file_command): Add extern declaration.
 >         * top.c (remote_shared_libs): New global variable.
 >         * top.h (remote_shared_libs): Likewise.
 > 
 > Index: main.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/main.c,v
 > retrieving revision 1.12
 > diff -u -p -r1.12 main.c
 > --- main.c 2001/07/14 18:59:07 1.12
 > +++ main.c 2001/07/19 21:46:15
 > @@ -264,6 +264,8 @@ captured_main (void *data)
 >        {"windows", no_argument, &use_windows, 1},
 >        {"statistics", no_argument, 0, 13},
 >        {"write", no_argument, &write_files, 1},
 > +      {"remote-shared-libs", no_argument, &remote_shared_libs, 1},
 > +      {"no-remote-shared-libs", no_argument, &remote_shared_libs, 0},
 >  /* Allow machine descriptions to add more options... */
 >  #ifdef ADDITIONAL_OPTIONS
 >        ADDITIONAL_OPTIONS
 > Index: remote.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/remote.c,v
 > retrieving revision 1.61
 > diff -u -p -r1.61 remote.c
 > --- remote.c 2001/07/17 01:23:44 1.61
 > +++ remote.c 2001/07/19 21:46:19
 > @@ -48,8 +48,9 @@
 >  #include "inf-loop.h"
 > 
 >  #include <signal.h>
 > +#include <string.h>
 >  #include "serial.h"
 > -
 > +#include "top.h"     /* for remote_shared_libs */
 >  #include "gdbcore.h" /* for exec_bfd */
 > 
 >  /* Prototypes for local functions */
 > @@ -202,6 +203,10 @@ static void show_packet_config_cmd (stru
 > 
 >  static void update_packet_config (struct packet_config *config);
 > 
 > +static void remote_get_list_of_shared_libraries(void);
 > +
 > +static char* find_file(char* basename);
 > +
 >  /* Define the target subroutine names */
 > 
 >  void open_remote_target (char *, int, struct target_ops *, int);
 > @@ -3061,7 +3066,9 @@ Packet Dropped");
 >     continue;
 >   }
 >      }
 > +
 >  got_status:
 > +  remote_get_list_of_shared_libraries();
 >    if (thread_num != -1)
 >      {
 >        return pid_to_ptid (thread_num);
 > @@ -3284,6 +3291,7 @@ Packet Dropped");
 >   }
 >      }
 >  got_status:
 > +  remote_get_list_of_shared_libraries();
 >    if (thread_num != -1)
 >      {
 >        return pid_to_ptid (thread_num);
 > @@ -6015,4 +6023,134 @@ Set use of remote protocol `Z' packets",
 >    add_cmd ("Z-packet", class_obscure, show_remote_protocol_Z_packet_cmd,
 >      "Show use of remote protocol `Z' packets ",
 >      &remote_show_cmdlist);
 > +}
 > +
 > +static void
 > +remote_get_list_of_shared_libraries (void)
 > +{
 > +  /*  This is a counter that gets used so that we don't run while the GDB is initializing */
 > +  static unsigned initializationBlock = 0;
 > +
 > +  /* We can't just check for a starting E for errors because the file name may start with one*/
 > +  char *error_string = "ENN: ";
 > +
 > +  char *buf = alloca (PBUFSIZ);
 > +  static char remote_does_not_suport = 0; /* If the remote doesn't support this, then we will
 > +                                             turn off this function */
 > +
 > +  /* Has the user asked for this feature:  Command line option: remote-shared-libs */
 > +  if( remote_shared_libs == 0 )
 > +     return;
 > +
 > +
 > +  /* The first times through, I don't believe we want to do this because gdb isn't completely initialized.
 > +     With out this flag add_symbol_file_command() hangs. */
 > +  if( initializationBlock < 2 )
 > +  {
 > +     ++initializationBlock;
 > +     return;
 > +  }
 > +
 > +  if ( remote_does_not_suport ) /* We've checked this and the stub doesn't support this functionality */
 > +  {
 > +     return;
 > +  }
 > +
 > +  putpkt ("qNewLibraries");
 > +  getpkt (buf, PBUFSIZ, 0);
 > +
 > +  if (buf[0] == '\000')
 > +     {
 > +       remote_does_not_suport = 1;  /* short circuit this function */
 > +       return;                      /* Return silently.  Stub doesn't support
 > +                                       this command. */
 > +     }
 > +
 > +  if (strncmp( buf, error_string, strlen(error_string)) == 0 )
 > +    {
 > +      warning ("Remote failure reply: %s", buf);
 > +      return;
 > +    }
 > +
 > +  if (buf[0] == '1') /* There are new shared libraries */
 > +    {
 > +        char *file = alloca (PBUFSIZ), *fqn;
 > +        int   address, values, first_space;
 > +
 > +        putpkt ("qLibraries");
 > +        getpkt (buf, PBUFSIZ, 0);
 > +
 > +        if (buf[0] == '\000')
 > +           {
 > +             remote_does_not_suport = 1;  /* short circuit this function */
 > +             return;                      /* Return silently.  Stub doesn't support
 > +                                             this command. */
 > +           }
 > +
 > +        if (strncmp( buf, error_string, strlen(error_string)) == 0 )
 > +           {
 > +             warning ("Remote failure reply: %s", buf);
 > +             return;
 > +           }
 > +
 > +        do
 > +           {   /*  buff should have the following layout:
 > +                      <filename> <textaddress> [-mapped] [-readnow] [-s <secname> <addr>]*
 > +                */
 > +               values = sscanf( buf, "%s %x", file, &address );
 > +               if( values < 2) break; /* check to make sure we have a minimum number of fields */
 > +               first_space = strlen(file);
 > +               if( (fqn = find_file (file) ) != 0 )
 > +               {
 > +                  strcpy( file, fqn );
 > +                  strcat( file, &buf[first_space] );
 > +                  add_symbol_file_command( file, 0 );
 > +               }
 > +
 > +               /* Get the next file from remote */
 > +               putpkt ("qLibraries");
 > +               getpkt (buf, PBUFSIZ, 0);
 > +
 > +               /* Check for errors*/
 > +               if ( strncmp( buf, error_string, strlen(error_string)) == 0 )
 > +                 {
 > +                   warning ("Remote failure reply: %s", buf);
 > +                   break;
 > +                 }
 > +           } while (buf[0] != '\000');
 > +
 > +    }
 > +  else if (buf[0] == '0')
 > +    {
 > +      /* There are no new shared libraries */
 > +    }
 > +  else
 > +    {
 > +      warning ("Remote reply is unrecognized: %s", buf);
 > +      return;
 > +    }
 > +
 > +  return;
 > +}
 > +
 > +static char*
 > +find_file (char* basename)
 > +{
 > +  int found_file;
 > +  static char* filename;
 > +  /* Search the $PATH environment variable. */
 > +  found_file = openp (getenv ("PATH"), 1, basename, O_RDONLY, 0, &filename);
 > +
 > +  /* If not found, next search $LD_LIBRARY_PATH environment variable. */
 > +  if (found_file < 0 )
 > +    found_file = openp (getenv ("LD_LIBRARY_PATH"), 1, basename, O_RDONLY, 0, &filename);
 > +
 > +  /* We really don't want the file open here, we just wanted the side effects of
 > +     this function call.  If the file was opened, close it. */
 > +  if (found_file >= 0 )
 > +    close( found_file );
 > +  else
 > +    return 0; /* Report that a file wasn't found */
 > +
 > +  return filename;
 >  }
 > Index: symfile.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symfile.c,v
 > retrieving revision 1.36
 > diff -u -p -r1.36 symfile.c
 > --- symfile.c 2001/07/15 18:57:06 1.36
 > +++ symfile.c 2001/07/19 21:46:20
 > @@ -112,8 +112,6 @@ static void load_command (char *, int);
 > 
 >  static void symbol_file_add_main_1 (char *args, int from_tty, int flags);
 > 
 > -static void add_symbol_file_command (char *, int);
 > -
 >  static void add_shared_symbol_files_command (char *, int);
 > 
 >  static void cashier_psymtab (struct partial_symtab *);
 > @@ -1410,7 +1408,7 @@ print_transfer_performance (struct ui_fi
 >     value to use. We are now discontinuing this type of ad hoc syntax. */
 > 
 >  /* ARGSUSED */
 > -static void
 > +void
 >  add_symbol_file_command (char *args, int from_tty)
 >  {
 >    char *filename = NULL;
 > Index: symfile.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symfile.h,v
 > retrieving revision 1.9
 > diff -u -p -r1.9 symfile.h
 > --- symfile.h 2001/03/06 08:21:17 1.9
 > +++ symfile.h 2001/07/19 21:46:20
 > @@ -280,6 +280,9 @@ extern void symbol_file_add_main (char *
 >  /* Clear GDB symbol tables. */
 >  extern void symbol_file_clear (int from_tty);
 > 
 > +/* Add a file to the symbol table */
 > +extern void add_symbol_file_command (char *args, int from_tty);
 > +
 >  /* From dwarfread.c */
 > 
 >  extern void
 > Index: top.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/top.c,v
 > retrieving revision 1.41
 > diff -u -p -r1.41 top.c
 > --- top.c 2001/07/17 17:25:14 1.41
 > +++ top.c 2001/07/19 21:46:21
 > @@ -165,6 +165,12 @@ int remote_debug = 0;
 >     target is off and running, which gdb is doing something else. */
 >  int target_executing = 0;
 > 
 > +/* This variable is added to control the loading of shared libraries
 > +   by a remote stub or by gdbserver.  The default is set to 0 so that
 > +   the behaviour will remain unchanged by default - i.e. we won't ask about
 > +   shared libraries.  */
 > +int remote_shared_libs = 0;
 > +
 >  /* Level of control structure.  */
 >  static int control_level;
 > 
 > Index: top.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/top.h,v
 > retrieving revision 1.5
 > diff -u -p -r1.5 top.h
 > --- top.h 2001/03/06 08:21:17 1.5
 > +++ top.h 2001/07/19 21:46:21
 > @@ -55,6 +55,7 @@ extern void set_prompt (char *);
 >  /* From random places.  */
 >  extern int mapped_symbol_files;
 >  extern int readnow_symbol_files;
 > +extern int remote_shared_libs;
 > 
 >  /* Perform _initialize initialization */
 >  extern void gdb_init (char *);
 >