Re: [PATCH v4] Implement $_exitsignal

[Pedro Alves <palves at redhat dot com>]

On 10/03/2013 06:48 AM, Sergio Durigan Junior wrote:
> On Tuesday, October 01 2013, Pedro Alves wrote:
>> "run" won't work with "target remote".  Please try the native-gdbserver.exp
>> board.
> 
> Ops, thanks for the catch.  I fixed it by checking for is_remote target
> and the presence of $use_gdb_stub.  I guess it's enough.  Tested on
> native-{,extended-,stdio-}gdbserver.

Sorry for not being clearer, but what I had in mind was the complete
opposite -- what would it take to make those tests _also_ run against
"target remote".  Is "run" special at all here?  The test just seems
to want to run the program to exit.  runto_main + gdb_continue_to_end
should work just as well for that.


>> (those clears could be put in a small helper, as they'll
>> be done in two places).
> 
> Done.  I was not sure where to put the declaration of this new helper
> function, so I decided for gdb/utils.h.  Not sure it's a good place,
> though.  Let me know what you think and I can change it if needed.

Not a good place.  utils.h == kitchen sink.  ;-)

infrun.c things are declared in inferior.h currently.



> +/* This function clears the convenience variables associated with the
> +   exit of the inferior.  Currently, those variables are $_exitcode
> +   and $_exitsignal.  */
> +
> +extern void clear_exit_convenience_vars (void);

It's more common to avoid the redundant "This function".  E.g.,:

/* Clear the convenience variables associated with the exit of the
   inferior.  Currently, those variables are $_exitcode and
   $_exitsignal.  */



> +	  else
> +	    {
> +	      static int have_warned_signal_to_target = 0;
> +
> +	      /* We don't have access to the target's method used for
> +		 converting between signal numbers (GDB's internal
> +		 representation <-> target's representation).
> +		 Therefore, we cannot do a good job at displaying this
> +		 information to the user.  It's better to just warn
> +		 her about it, and give up.  */
> +	      if (!have_warned_signal_to_target)
> +		{
> +		  have_warned_signal_to_target = 1;
> +		  warning (_("\
> +Cannot fill $_exitsignal with the correct signal number."));
> +		}

I thing this warning will be a nuisance for most targets.
_Everyone_ not debugging Linux will see this.  But, there's really
nothing a regular user or integrator can do to fix this, and I don't
believe this is such an important feature that warrants printing
this on most runs.  I'd rather see this downgraded to a
complaint() or debug output.


I recalled that signal tests should be skipped if
[target_info exists gdb,nosignals].


Also:

$ git am /tmp/mbox
Applying: Implement $_exitsignal
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:281: trailing whitespace.
#
warning: 1 line adds whitespace errors.

-- 
Pedro Alves