Re: RFA: [symfile.c} Fix to symbol_file_add()

[Fernando Nasser <fnasser at redhat dot com>]

Thanks for reviewing my patch so promptly Elena.  Please see my comments
below.

Elena Zannoni wrote:
> 
> Fernando Nasser writes:
>  > Paul N. Hilfinger has pointed out to me that a few operations should be
>  > done every time a new symbol file is read.  This was an oversight in a
>  > patch I wrote in January.  The small patch attached fixes that.
>  >
>  > ChangeLog:
>  >
>  >      * symfile.c (symbol_file_command): Move cleanup operations
>  >      from here...
>  >      (symbol_file_add): ...to here, so they are run every time a new
>  >      symbol file is read.
>  >
> 
> This would change the behavior of all the calls to symbol_file_add().
> Which may not be what you want.


You are right, this bits:
>  > + #ifdef HPUXHPPA
>  > +   RESET_HP_UX_GLOBALS ();
>  > + #endif
>  > +
>  > +   set_initial_language ();

should only be done when a new "main" is loaded.  I suggested moving it
to symbol_file_add_main(), which is called exactly in those cases.

However, the bit:
>  > +   /* Getting new symbols may change our opinion about
>  > +      what is frameless.  */
>  > +   reinit_frame_cache ();

should _always_ be done, whenever we load new symbols.

What happens today, and happened _before_ my patch (it hasn't changed
any call to symbol_file_add() nor symbol_file_add() itself except for
the clear part), is that _some_ of the callers do it and some do not.

I guess this will be very difficult to control unless we move the
reinit_frame_cache () call to inside symbol_file_add().

So, I would, if you agree, post a patch (and ask Paul to help me test
it) that does:

1) Move the HP and set_initial_language() bits to symbol_file_add_main()

2) Move the reinit_frame_cache () bit to symbol_file_add(), as the
current patch does.

3) Remove the duplicate call to reinit_frame_cache () from the targets.


How does it sound?


Regards,
Fernando



> Later on in symfile.c there is another
> call to symbol_file_add which is followed by reinit_frame_cache(). In
> this case the same thing would be done twice, and in other calls it
> would be done, when it was not done before.  I am not sure why the
> other calls to symbol_file_add don't reinititalize things, but it
> seems a little too risky to do this, w/o making sure those platforms
> are not affected.
> 
> coff-solib.c:91:          objfile = symbol_file_add (filename, from_tty,
> cxux-nat.c:370:  objfile = symbol_file_add (LIBC_FILE, 0, NULL, 0, OBJF_READNOW);
> cxux-nat.c:390:       symbol_file_add (path_name, 1, &section_addrs, 0, 0);
> irix5-nat.c:850:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
> osfsolib.c:585:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
> pa64solib.c:273:  so->objfile = symbol_file_add (name, from_tty, &section_addrs, 0, OBJF_SHARED);
> remote-udi.c:1291:  symbol_file_add (strtok (args, " \t"), from_tty, NULL, 1, 0);
> remote-vx.c:670:  objfile = symbol_file_add (name, from_tty, NULL, 0, 0);
> solib.c:313:  so->objfile = symbol_file_add (so->so_name, so->from_tty,
> somsolib.c:290:  so->objfile = symbol_file_add (name, from_tty, NULL, 0, OBJF_SHARED);
> symfile.c:905:  symbol_file_add (args, from_tty, NULL, 1, 0);
> symfile.c:982:            symbol_file_add (name, from_tty, NULL, 1, flags);
> symfile.c:1549:  symbol_file_add (filename, from_tty, &section_addrs, 0, flags);
> win32-nat.c:444:  p->ret = symbol_file_add (p->name, p->from_tty, p->addrs, p->mainline, p->flags);
> 
> Since the original code substitution was to replace calls to
> symbol_file_command() with symbol_file_add_main(), I would think that
> the lines below should be left in the symbol_file_command function and
> added to symbol_file_add_main() instead, therefore preserving the
> original semantics.
> 
> Elena
> 
>  > --
>  > Fernando Nasser
>  > Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
>  > 2323 Yonge Street, Suite #300
>  > Toronto, Ontario   M4P 2C9Index: symfile.c
>  > ===================================================================
>  > RCS file: /cvs/src/src/gdb/symfile.c,v
>  > retrieving revision 1.31
>  > diff -c -p -r1.31 symfile.c
>  > *** symfile.c        2001/04/05 02:02:13     1.31
>  > --- symfile.c        2001/04/29 16:13:12
>  > *************** symbol_file_add (char *name, int from_tt
>  > *** 893,898 ****
>  > --- 893,907 ----
>  >     if (target_new_objfile_hook)
>  >       target_new_objfile_hook (objfile);
>  >
>  > + #ifdef HPUXHPPA
>  > +   RESET_HP_UX_GLOBALS ();
>  > + #endif
>  > +   /* Getting new symbols may change our opinion about
>  > +      what is frameless.  */
>  > +   reinit_frame_cache ();
>  > +
>  > +   set_initial_language ();
>  > +
>  >     return (objfile);
>  >   }
>  >
>  > *************** symbol_file_command (char *args, int fro
>  > *** 980,993 ****
>  >              {
>  >                     name = *argv;
>  >                symbol_file_add (name, from_tty, NULL, 1, flags);
>  > - #ifdef HPUXHPPA
>  > -              RESET_HP_UX_GLOBALS ();
>  > - #endif
>  > -              /* Getting new symbols may change our opinion about
>  > -                 what is frameless.  */
>  > -              reinit_frame_cache ();
>  > -
>  > -              set_initial_language ();
>  >              }
>  >        argv++;
>  >      }
>  > --- 989,994 ----

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9