Re: [PATCH 2/2] target_set_syscall_catchpoint, use gdb::array_view and bool

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2017-10-30 11:32, Pedro Alves wrote:
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index 1cbe6f8..8702a9e 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -1286,36 +1286,34 @@ debug_follow_exec (struct target_ops *self,
> struct inferior *arg1, char *arg2)
>  }
>
>  static int
> -delegate_set_syscall_catchpoint (struct target_ops *self, int arg1,
> int arg2, int arg3, int arg4, int *arg5)
> +delegate_set_syscall_catchpoint (struct target_ops *self, int arg1,
> bool arg2, int arg3, gdb::array_view<const int> arg4)
>  {
>    self = self->beneath;
> -  return self->to_set_syscall_catchpoint (self, arg1, arg2, arg3, 
> arg4, arg5);
> +  return self->to_set_syscall_catchpoint (self, arg1, arg2, arg3, 
> arg4);
>  }
>
>  static int
> -tdefault_set_syscall_catchpoint (struct target_ops *self, int arg1,
> int arg2, int arg3, int arg4, int *arg5)
> +tdefault_set_syscall_catchpoint (struct target_ops *self, int arg1,
> bool arg2, int arg3, gdb::array_view<const int> arg4)
>  {
>    return 1;
>  }
>
>  static int
> -debug_set_syscall_catchpoint (struct target_ops *self, int arg1, int
> arg2, int arg3, int arg4, int *arg5)
> +debug_set_syscall_catchpoint (struct target_ops *self, int arg1, bool
> arg2, int arg3, gdb::array_view<const int> arg4)
>  {
>    int result;
>    fprintf_unfiltered (gdb_stdlog, "-> %s->to_set_syscall_catchpoint
> (...)\n", debug_target.to_shortname);
> -  result = debug_target.to_set_syscall_catchpoint (&debug_target,
> arg1, arg2, arg3, arg4, arg5);
> +  result = debug_target.to_set_syscall_catchpoint (&debug_target,
> arg1, arg2, arg3, arg4);
>    fprintf_unfiltered (gdb_stdlog, "<- %s->to_set_syscall_catchpoint
> (", debug_target.to_shortname);
>    target_debug_print_struct_target_ops_p (&debug_target);
>    fputs_unfiltered (", ", gdb_stdlog);
>    target_debug_print_int (arg1);
>    fputs_unfiltered (", ", gdb_stdlog);
> -  target_debug_print_int (arg2);
> +  target_debug_print_bool (arg2);
>    fputs_unfiltered (", ", gdb_stdlog);
>    target_debug_print_int (arg3);
>    fputs_unfiltered (", ", gdb_stdlog);
> -  target_debug_print_int (arg4);
> -  fputs_unfiltered (", ", gdb_stdlog);
> -  target_debug_print_int_p (arg5);
> +  target_debug_print_gdb_array_view_const_int (arg4);
>    fputs_unfiltered (") = ", gdb_stdlog);
>    target_debug_print_int (result);
>    fputs_unfiltered ("\n", gdb_stdlog);
> diff --git a/gdb/target.h b/gdb/target.h
> index e7577e1..7bcdefb 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -612,7 +612,8 @@ struct target_ops
>      void (*to_follow_exec) (struct target_ops *, struct inferior *, 
> char *)
>        TARGET_DEFAULT_IGNORE ();
>      int (*to_set_syscall_catchpoint) (struct target_ops *,
> -				      int, int, int, int, int *)
> +				      int, bool, int,
> +				      gdb::array_view<const int>)

Is there any advantage of using array_view instead of vector directly?  
In case we decide to change from vector to another "kind" of array?

>        TARGET_DEFAULT_RETURN (1);
>      int (*to_has_exited) (struct target_ops *, int, int, int *)
>        TARGET_DEFAULT_RETURN (0);
> @@ -1620,28 +1621,24 @@ void target_follow_exec (struct inferior *inf,
> char *execd_pathname);
>
>  /* Syscall catch.
>
> -   NEEDED is nonzero if any syscall catch (of any kind) is requested.
> -   If NEEDED is zero, it means the target can disable the mechanism to
> +   NEEDED is true if any syscall catch (of any kind) is requested.
> +   If NEEDED is false, it means the target can disable the mechanism 
> to
>     catch system calls because there are no more catchpoints of this 
> type.
>
>     ANY_COUNT is nonzero if a generic (filter-less) syscall catch is
> -   being requested.  In this case, both TABLE_SIZE and TABLE should
> -   be ignored.
> +   being requested.  In this case, SYSCALL_COUNTS should be ignored.
>
> -   TABLE_SIZE is the number of elements in TABLE.  It only matters if
> -   ANY_COUNT is zero.
> -
> -   TABLE is an array of ints, indexed by syscall number.  An element 
> in
> -   this array is nonzero if that syscall should be caught.  This 
> argument
> -   only matters if ANY_COUNT is zero.
> +   SYSCALL_COUNTS is an array of ints, indexed by syscall number.  An
> +   element in this array is nonzero if that syscall should be caught.
> +   This argument only matters if ANY_COUNT is zero.
>
>     Return 0 for success, 1 if syscall catchpoints are not supported or 
> -1
>     for failure.  */
>
> -#define target_set_syscall_catchpoint(pid, needed, any_count,
> table_size, table) \
> +#define target_set_syscall_catchpoint(pid, needed, any_count, table) \
>       (*current_target.to_set_syscall_catchpoint) (&current_target,	\
>  						  pid, needed, any_count, \
> -						  table_size, table)
> +						  table)

The doc talks about SYSCALL_COUNTS, but the parameter is named table.

Simon