Re: [PATCH v2 2/3] Add an optional "alias" attribute to syscall entries.

[Pedro Alves <palves@redhat.com>]

Hi,

On 11/06/2018 05:54 PM, John Baldwin wrote:

> --- a/gdb/syscalls/gdb-syscalls.dtd
> +++ b/gdb/syscalls/gdb-syscalls.dtd
> @@ -12,4 +12,5 @@
>  <!ATTLIST syscall
>  	name			CDATA	#REQUIRED
>  	number			CDATA	#REQUIRED
> +	alias			CDATA	#IMPLIED
>  	groups			CDATA	#IMPLIED>

I think there should be a NEWS entry for this.

I'd think there should be a docs update as well, but it
looks like this dtd isn't documented in the manual.  :-/


> +	  /* We have a name.  Let's check if it's valid and fetch a
> +	     list of matching numbers.  */
> +	  std::vector<int> numbers = get_syscalls_by_name (gdbarch, cur_name);
>  
> -	  if (s.number == UNKNOWN_SYSCALL)
> +	  if (numbers.empty ())
>  	    /* Here we have to issue an error instead of a warning,
>  	       because GDB cannot do anything useful if there's no
>  	       syscall number to be caught.  */
>  	    error (_("Unknown syscall name '%s'."), cur_name);
>  
>  	  /* Ok, it's valid.  */
> -	  result.push_back (s.number);
> +	  for (int number : numbers)
> +	    result.push_back (number);

Nit: this return-vector interface seems like leads to a bit of unnecessary
work, and double the number of necessary vector/heap allocations -- if
get_syscalls_by_name were passed RESULT instead of returning a new vector,
it could add to RESULT directly?  The code above would like something like this
instead with get_syscalls_by_name returning true/false to indicate
whether something was added:

	  /* We have a name.  Let's check if it's valid and fetch a
	     list of matching numbers.  */
	  if (!get_syscalls_by_name (gdbarch, cur_name, result))
 	    /* Here we have to issue an error instead of a warning,
	       because GDB cannot do anything useful if there's no
	       syscall number to be caught.  */
  	    error (_("Unknown syscall name '%s'."), cur_name);

That means there's no need to copy numbers between the vectors
(since there's only one vector).

Same for get_syscalls_by_group.

Note that instead of:

> +	  for (int number : numbers)
> +	    result.push_back (number);

You could write:

  result.insert (result.end (), numbers.begin (), numbers.end ());

which may be more efficient assuming insert is smart enough
to call std::vector::reserve to allocate space in one go.

Thanks,
Pedro Alves