RE: [PATCH v3 1/1] gdb, infcmd: Support jump command with same line in multiple symtabs

["Puputti, Matti" <matti.puputti@intel.com>]

Hi Andrew,

Thank you for your comments.
I plan to follow all your suggestions in the next patch.

On the document update, what you think about this proposal:
"If @var{locspec} resolves to more than one address,
those outside the current compilation unit are ignored.  If still not a
unique address found, the command aborts before jumping. "

Br,
Matti Puputti

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Friday, April 28, 2023 4:40 PM
> To: Puputti, Matti <matti.puputti@intel.com>; gdb-patches@sourceware.org
> Cc: blarsen@redhat.com
> Subject: Re: [PATCH v3 1/1] gdb, infcmd: Support jump command with same
> line in multiple symtabs
> 

< . . . >

> > +      sals.erase (std::remove_if (sals.begin (), sals.end (),
> > +		  [] (symtab_and_line &sal)
> > +		    {
> > +		      struct symtab_and_line cursal
> > +			  = get_current_source_symtab_and_line ();
> > +		      return sal.symtab != cursal.symtab;
> 
> Maybe worrying about nothing, but I wonder if we should move the call to
> get_current_source_symtab_and_line outside of the lambda, and then
> capture cursal by reference.
> 
> Also, the 'symtab_and_line &sal' can be made const.
> 
> > +		    }), sals.end ());
> > +      if (sals.size () != 1)
> > +	error (_("Unreasonable jump request"));
> > +    }
> 
> This all makes sense, but I think you need to update the docs.  The docs
> currently say:
> 
>   ... If @var{locspec} resolves to more than one address, the command
>   aborts before jumping.
> 
> Which isn't true any more right?
> 

< . . . >

> > +
> > +standard_testfile .c
> > +set srcfile2 jump_multiple_objfiles_foo.c
> > +set srcfile3 jump_multiple_objfiles.h
> 
> If you rename 'jump_multiple_objfiles_foo.c' to
> 'jump_multiple_objfiles-foo.c', then you can replace these three lines
> with:
> 
>   standard_testfile .c -foo.c .h
> 
> which is the preferred approach these days.
> 

< . . . >

> > +
> > +if { ![runto_main] } {
> > +    perror "couldn't run to breakpoint"
> 
> Drop this perror call.  runto_main will FAIL if appropriate.
> 
> Thanks,
> Andrew
> 

< . . . >
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928