[v2,1/4] Implemement support for groups of syscalls in the xml-syscall interface.

  Hello,

These are the updated patches after applying Sergio's suggestions.

Is this good to go?

gdb/

	* syscalls/gdb-syscalls.dtd: Include group attribute to the
	syscall element.
	* xml-syscall.c (get_syscalls_by_group): New.
	(get_syscall_group_names): New.
	(struct syscall_group_desc): New structure to store group data.
	(struct syscalls_info): Include field to store the group list.
	(syscalls_info_free_syscall_group_desc): New.
	(free_syscalls_info): Free group list.
	(syscall_group_create_syscall_group_desc): New.
	(syscall_group_add_syscall): New.
	(syscall_create_syscall_desc): Add syscall to its groups.
	(syscall_start_syscall): Load group attribute.
	(syscall_group_get_group_by_name): New.
	(xml_list_syscalls_by_group): New.
	(xml_list_of_groups): New.
	* xml-syscall.h (get_syscalls_by_group): Export function
	to retrieve a list of syscalls filtered by the group name.
	(get_syscall_group_names): Export function to retrieve the list
	of syscall groups.
---
 gdb/syscalls/gdb-syscalls.dtd |   3 +-
 gdb/xml-syscall.c             | 230 +++++++++++++++++++++++++++++++++++++++++-
 gdb/xml-syscall.h             |  15 +++
 3 files changed, 245 insertions(+), 3 deletions(-)
  

Gabriel Krisman Bertazi <gabriel@krisman.be> writes:

> Hello,
>
> These are the updated patches after applying Sergio's suggestions.
>
> Is this good to go?
>

Ping. :)

> gdb/
>
> 	* syscalls/gdb-syscalls.dtd: Include group attribute to the
> 	syscall element.
> 	* xml-syscall.c (get_syscalls_by_group): New.
> 	(get_syscall_group_names): New.
> 	(struct syscall_group_desc): New structure to store group data.
> 	(struct syscalls_info): Include field to store the group list.
> 	(syscalls_info_free_syscall_group_desc): New.
> 	(free_syscalls_info): Free group list.
> 	(syscall_group_create_syscall_group_desc): New.
> 	(syscall_group_add_syscall): New.
> 	(syscall_create_syscall_desc): Add syscall to its groups.
> 	(syscall_start_syscall): Load group attribute.
> 	(syscall_group_get_group_by_name): New.
> 	(xml_list_syscalls_by_group): New.
> 	(xml_list_of_groups): New.
> 	* xml-syscall.h (get_syscalls_by_group): Export function
> 	to retrieve a list of syscalls filtered by the group name.
> 	(get_syscall_group_names): Export function to retrieve the list
> 	of syscall groups.
> ---
>  gdb/syscalls/gdb-syscalls.dtd |   3 +-
>  gdb/xml-syscall.c             | 230 +++++++++++++++++++++++++++++++++++++++++-
>  gdb/xml-syscall.h             |  15 +++
>  3 files changed, 245 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/syscalls/gdb-syscalls.dtd b/gdb/syscalls/gdb-syscalls.dtd
> index 3ad3625..b60eb74 100644
> --- a/gdb/syscalls/gdb-syscalls.dtd
> +++ b/gdb/syscalls/gdb-syscalls.dtd
> @@ -11,4 +11,5 @@
>  <!ELEMENT syscall		EMPTY>
>  <!ATTLIST syscall
>  	name			CDATA	#REQUIRED
> -	number			CDATA	#REQUIRED>
> +	number			CDATA	#REQUIRED
> +	groups			CDATA	#OPTIONAL>
> diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
> index 987fe7a..0d11c3a 100644
> --- a/gdb/xml-syscall.c
> +++ b/gdb/xml-syscall.c
> @@ -77,6 +77,20 @@ get_syscall_names (struct gdbarch *gdbarch)
>    return NULL;
>  }
>  
> +struct syscall *
> +get_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
> +{
> +  syscall_warn_user ();
> +  return NULL;
> +}
> +
> +const char **
> +get_syscall_group_names (struct gdbarch *gdbarch)
> +{
> +  syscall_warn_user ();
> +  return NULL;
> +}
> +
>  #else /* ! HAVE_LIBEXPAT */
>  
>  /* Structure which describes a syscall.  */
> @@ -92,6 +106,19 @@ typedef struct syscall_desc
>  } *syscall_desc_p;
>  DEF_VEC_P(syscall_desc_p);
>  
> +/* Structure of a syscall group.  */
> +typedef struct syscall_group_desc
> +{
> +  /* The group name.  */
> +
> +  char *name;
> +
> +  /* The syscalls that are part of the group.  */
> +
> +  VEC(syscall_desc_p) *syscalls;
> +} *syscall_group_desc_p;
> +DEF_VEC_P(syscall_group_desc_p);
> +
>  /* Structure that represents syscalls information.  */
>  struct syscalls_info
>  {
> @@ -99,6 +126,10 @@ struct syscalls_info
>  
>    VEC(syscall_desc_p) *syscalls;
>  
> +  /* The syscall groups.  */
> +
> +  VEC(syscall_group_desc_p) *groups;
> +
>    /* Variable that will hold the last known data-directory.  This is
>       useful to know whether we should re-read the XML info for the
>       target.  */
> @@ -126,11 +157,21 @@ syscalls_info_free_syscalls_desc (struct syscall_desc *sd)
>    xfree (sd->name);
>  }
>  
> +/* Free syscall_group_desc members but not the structure itself.  */
> +
> +static void
> +syscalls_info_free_syscall_group_desc (struct syscall_group_desc *sd)
> +{
> +  VEC_free (syscall_desc_p, sd->syscalls);
> +  xfree (sd->name);
> +}
> +
>  static void
>  free_syscalls_info (void *arg)
>  {
>    struct syscalls_info *syscalls_info = arg;
>    struct syscall_desc *sysdesc;
> +  struct syscall_group_desc *groupdesc;
>    int i;
>  
>    xfree (syscalls_info->my_gdb_datadir);
> @@ -143,6 +184,15 @@ free_syscalls_info (void *arg)
>  	syscalls_info_free_syscalls_desc (sysdesc);
>        VEC_free (syscall_desc_p, syscalls_info->syscalls);
>      }
> +  if (syscalls_info->groups != NULL)
> +    {
> +      for (i = 0;
> +	   VEC_iterate (syscall_group_desc_p,
> +			syscalls_info->groups, i, groupdesc);
> +	   i++)
> +	syscalls_info_free_syscall_group_desc (groupdesc);
> +      VEC_free (syscall_group_desc_p, syscalls_info->groups);
> +    }
>  
>    xfree (syscalls_info);
>  }
> @@ -153,16 +203,71 @@ make_cleanup_free_syscalls_info (struct syscalls_info *syscalls_info)
>    return make_cleanup (free_syscalls_info, syscalls_info);
>  }
>  
> +/* Create a new syscall group.  Return pointer to the
> +   syscall_group_desc structure that represents the new group.  */
> +
> +static struct syscall_group_desc *
> +syscall_group_create_syscall_group_desc (struct syscalls_info *syscalls_info,
> +					 const char *group)
> +{
> +  struct syscall_group_desc *groupdesc = XCNEW (struct syscall_group_desc);
> +
> +  groupdesc->name = xstrdup (group);
> +
> +  VEC_safe_push (syscall_group_desc_p, syscalls_info->groups, groupdesc);
> +
> +  return groupdesc;
> +}
> +
> +/* Add a syscall to the group.  If group doesn't exist, create
> +   it.  */
> +
> +static void
> +syscall_group_add_syscall (struct syscalls_info *syscalls_info,
> +			   struct syscall_desc *syscall,
> +			   const char *group)
> +{
> +  struct syscall_group_desc *groupdesc;
> +  int i;
> +
> +  /* Search for an existing group.  */
> +  for (i = 0;
> +       VEC_iterate (syscall_group_desc_p,
> +		    syscalls_info->groups, i, groupdesc);
> +       i++)
> +    if (strcmp (groupdesc->name, group) == 0)
> +      break;
> +
> +  if (groupdesc == NULL)
> +    {
> +      /* No group was found with this name.  We must create a new
> +	 one.  */
> +      groupdesc = syscall_group_create_syscall_group_desc (syscalls_info,
> +							   group);
> +    }
> +
> +  VEC_safe_push (syscall_desc_p, groupdesc->syscalls, syscall);
> +}
> +
>  static void
>  syscall_create_syscall_desc (struct syscalls_info *syscalls_info,
> -                             const char *name, int number)
> +                             const char *name, int number,
> +			     char *groups)
>  {
>    struct syscall_desc *sysdesc = XCNEW (struct syscall_desc);
> +  char *group;
>  
>    sysdesc->name = xstrdup (name);
>    sysdesc->number = number;
>  
>    VEC_safe_push (syscall_desc_p, syscalls_info->syscalls, sysdesc);
> +
> +  /*  Add syscall to its groups.  */
> +  if (groups != NULL)
> +    for (group = strtok (groups, ",");
> +	 group != NULL;
> +	 group = strtok (NULL, ","))
> +      syscall_group_add_syscall (syscalls_info, sysdesc, group);
>  }
>  
>  /* Handle the start of a <syscall> element.  */
> @@ -177,6 +282,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
>    /* syscall info.  */
>    char *name = NULL;
>    int number = 0;
> +  char *groups = NULL;
>  
>    len = VEC_length (gdb_xml_value_s, attributes);
>  
> @@ -186,13 +292,16 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
>          name = attrs[i].value;
>        else if (strcmp (attrs[i].name, "number") == 0)
>          number = * (ULONGEST *) attrs[i].value;
> +      else if (strcmp (attrs[i].name, "groups") == 0)
> +        groups = attrs[i].value;
>        else
>          internal_error (__FILE__, __LINE__,
>                          _("Unknown attribute name '%s'."), attrs[i].name);
>      }
>  
>    gdb_assert (name);
> -  syscall_create_syscall_desc (data->syscalls_info, name, number);
> +
> +  syscall_create_syscall_desc (data->syscalls_info, name, number, groups);
>  }
>  
>  
> @@ -200,6 +309,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
>  static const struct gdb_xml_attribute syscall_attr[] = {
>    { "number", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
>    { "name", GDB_XML_AF_NONE, NULL, NULL },
> +  { "groups", GDB_XML_AF_OPTIONAL, NULL, NULL },
>    { NULL, GDB_XML_AF_NONE, NULL, NULL }
>  };
>  
> @@ -321,6 +431,33 @@ init_syscalls_info (struct gdbarch *gdbarch)
>    set_gdbarch_syscalls_info (gdbarch, syscalls_info);
>  }
>  
> +/* Search for a syscall group by its name.  Return syscall_group_desc
> +   structure for the group if found or NULL otherwise.  */
> +
> +static struct syscall_group_desc *
> +syscall_group_get_group_by_name (const struct syscalls_info *syscalls_info,
> +				 const char *group)
> +{
> +  struct syscall_group_desc *groupdesc;
> +  int i;
> +
> +  if (syscalls_info == NULL)
> +    return NULL;
> +
> +  if (group == NULL)
> +    return NULL;
> +
> +   /* Search for existing group.  */
> +  for (i = 0;
> +       VEC_iterate (syscall_group_desc_p,
> +		    syscalls_info->groups, i, groupdesc);
> +       i++)
> +    if (strcmp (groupdesc->name, group) == 0)
> +      return groupdesc;
> +
> +  return NULL;
> +}
> +
>  static int
>  xml_get_syscall_number (struct gdbarch *gdbarch,
>                          const char *syscall_name)
> @@ -388,6 +525,75 @@ xml_list_of_syscalls (struct gdbarch *gdbarch)
>    return names;
>  }
>  
> +/*  Iterate over the syscall_group_desc element to return a list of
> +    syscalls that are part of the given group, terminated by an empty
> +    element.  If the syscall group doesn't exist, return NULL.  */
> +
> +static struct syscall *
> +xml_list_syscalls_by_group (struct gdbarch *gdbarch,
> +			    const char *group)
> +{
> +  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
> +  struct syscall_group_desc *groupdesc;
> +  struct syscall_desc *sysdesc;
> +  struct syscall *syscalls = NULL;
> +  int nsyscalls;
> +  int i;
> +
> +  if (syscalls_info == NULL)
> +    return NULL;
> +
> +  groupdesc = syscall_group_get_group_by_name (syscalls_info, group);
> +  if (groupdesc == NULL)
> +    return NULL;
> +
> +  nsyscalls = VEC_length (syscall_desc_p, groupdesc->syscalls);
> +  syscalls = xmalloc ((nsyscalls + 1) * sizeof (struct syscall));
> +
> +  for (i = 0;
> +       VEC_iterate (syscall_desc_p, groupdesc->syscalls, i, sysdesc);
> +       i++)
> +    {
> +      syscalls[i].name = sysdesc->name;
> +      syscalls[i].number = sysdesc->number;
> +    }
> +
> +  /* Add final element marker.  */
> +  syscalls[i].name = NULL;
> +  syscalls[i].number = 0;
> +
> +  return syscalls;
> +}
> +
> +/* Return the list of syscall groups.  If no syscall group is
> +   available, return NULL.  */
> +
> +static const char **
> +xml_list_of_groups (struct gdbarch *gdbarch)
> +{
> +  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
> +  struct syscall_group_desc *groupdesc;
> +  const char **names = NULL;
> +  int i;
> +  int ngroups;
> +
> +  if (syscalls_info == NULL)
> +    return NULL;
> +
> +  ngroups = VEC_length (syscall_group_desc_p, syscalls_info->groups);
> +  names = xmalloc ((ngroups + 1) * sizeof (char *));
> +
> +  for (i = 0;
> +       VEC_iterate (syscall_group_desc_p,
> +		    syscalls_info->groups, i, groupdesc);
> +       i++)
> +    names[i] = groupdesc->name;
> +
> +  names[i] = NULL;
> +
> +  return names;
> +}
> +
>  void
>  set_xml_syscall_file_name (struct gdbarch *gdbarch, const char *name)
>  {
> @@ -422,4 +628,24 @@ get_syscall_names (struct gdbarch *gdbarch)
>    return xml_list_of_syscalls (gdbarch);
>  }
>  
> +/* See comment in xml-syscall.h.  */
> +
> +struct syscall *
> +get_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
> +{
> +  init_syscalls_info (gdbarch);
> +
> +  return xml_list_syscalls_by_group (gdbarch, group);
> +}
> +
> +/* See comment in xml-syscall.h.  */
> +
> +const char **
> +get_syscall_group_names (struct gdbarch *gdbarch)
> +{
> +  init_syscalls_info (gdbarch);
> +
> +  return xml_list_of_groups (gdbarch);
> +}
> +
>  #endif /* ! HAVE_LIBEXPAT */
> diff --git a/gdb/xml-syscall.h b/gdb/xml-syscall.h
> index 42b9dea..9ba4e93 100644
> --- a/gdb/xml-syscall.h
> +++ b/gdb/xml-syscall.h
> @@ -50,4 +50,19 @@ void get_syscall_by_name (struct gdbarch *gdbarch,
>  
>  const char **get_syscall_names (struct gdbarch *gdbarch);
>  
> +/* Function used to retrieve the list of syscalls of a given group in
> +   the system.  Return a list of syscalls that are element of the
> +   group, terminated by an empty element. The list is malloc'ed
> +   and must be freed by the caller.  If group doesn't exist, return
> +   NULL.  */
> +
> +struct syscall *get_syscalls_by_group (struct gdbarch *gdbarch,
> +				       const char *group);
> +
> +/* Function used to retrieve the list of syscall groups in the system.
> +   Return an array of strings terminated by a NULL element.  The list
> +   must be freed by the caller.  */
> +
> +const char **get_syscall_group_names (struct gdbarch *gdbarch);
> +
>  #endif /* XML_SYSCALL_H */
  

Gabriel Krisman Bertazi <gabriel@krisman.be> writes:

>> Is this good to go?
>>
>
> Ping. :)

ping^2
  

Gabriel Krisman Bertazi <gabriel@krisman.be> writes:

> Gabriel Krisman Bertazi <gabriel@krisman.be> writes:
>
>>> Is this good to go?
>>>
>>
>> Ping. :)
>
> ping^2

ping^3

Heh. Missed a few weekly pings.  Do you guys think we might get this
upstream anytime soon?

Thanks,
  

Gabriel Krisman Bertazi writes:
 > Gabriel Krisman Bertazi <gabriel@krisman.be> writes:
 > 
 > > Gabriel Krisman Bertazi <gabriel@krisman.be> writes:
 > >
 > >>> Is this good to go?
 > >>>
 > >>
 > >> Ping. :)
 > >
 > > ping^2
 > 
 > ping^3
 > 
 > Heh. Missed a few weekly pings.  Do you guys think we might get this
 > upstream anytime soon?
 > 
 > Thanks,
 > 
 > -- 
 > Gabriel Krisman Bertazi

Hi.

Sorry for the delay.

It's on my list to get reviewed this week.
Please ping me directly if you don't hear from me by Thursday.

[And thanks Sergio for the heads up!]
  

Gabriel Krisman Bertazi <gabriel@krisman.be> writes:
> Hello,
>
> These are the updated patches after applying Sergio's suggestions.
>
> Is this good to go?

Hi.
I didn't make sure that I'm not repeating something Sergio mentioned.

Several nits inline.
Ok with those changes (and those mentioned in other reviews).

>
> gdb/
>
> 	* syscalls/gdb-syscalls.dtd: Include group attribute to the
> 	syscall element.
> 	* xml-syscall.c (get_syscalls_by_group): New.
> 	(get_syscall_group_names): New.
> 	(struct syscall_group_desc): New structure to store group data.
> 	(struct syscalls_info): Include field to store the group list.
> 	(syscalls_info_free_syscall_group_desc): New.
> 	(free_syscalls_info): Free group list.
> 	(syscall_group_create_syscall_group_desc): New.
> 	(syscall_group_add_syscall): New.
> 	(syscall_create_syscall_desc): Add syscall to its groups.
> 	(syscall_start_syscall): Load group attribute.
> 	(syscall_group_get_group_by_name): New.
> 	(xml_list_syscalls_by_group): New.
> 	(xml_list_of_groups): New.
> 	* xml-syscall.h (get_syscalls_by_group): Export function
> 	to retrieve a list of syscalls filtered by the group name.
> 	(get_syscall_group_names): Export function to retrieve the list
> 	of syscall groups.
> ---
>  gdb/syscalls/gdb-syscalls.dtd |   3 +-
>  gdb/xml-syscall.c             | 230 +++++++++++++++++++++++++++++++++++++++++-
>  gdb/xml-syscall.h             |  15 +++
>  3 files changed, 245 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/syscalls/gdb-syscalls.dtd b/gdb/syscalls/gdb-syscalls.dtd
> index 3ad3625..b60eb74 100644
> --- a/gdb/syscalls/gdb-syscalls.dtd
> +++ b/gdb/syscalls/gdb-syscalls.dtd
> @@ -11,4 +11,5 @@
>  <!ELEMENT syscall		EMPTY>
>  <!ATTLIST syscall
>  	name			CDATA	#REQUIRED
> -	number			CDATA	#REQUIRED>
> +	number			CDATA	#REQUIRED
> +	groups			CDATA	#OPTIONAL>
> diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
> index 987fe7a..0d11c3a 100644
> --- a/gdb/xml-syscall.c
> +++ b/gdb/xml-syscall.c
> @@ -77,6 +77,20 @@ get_syscall_names (struct gdbarch *gdbarch)
>    return NULL;
>  }
>  
> +struct syscall *
> +get_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
> +{
> +  syscall_warn_user ();
> +  return NULL;
> +}
> +
> +const char **
> +get_syscall_group_names (struct gdbarch *gdbarch)
> +{
> +  syscall_warn_user ();
> +  return NULL;
> +}
> +
>  #else /* ! HAVE_LIBEXPAT */
>  
>  /* Structure which describes a syscall.  */
> @@ -92,6 +106,19 @@ typedef struct syscall_desc
>  } *syscall_desc_p;
>  DEF_VEC_P(syscall_desc_p);
>  
> +/* Structure of a syscall group.  */
> +typedef struct syscall_group_desc
> +{
> +  /* The group name.  */
> +
> +  char *name;
> +
> +  /* The syscalls that are part of the group.  */
> +
> +  VEC(syscall_desc_p) *syscalls;
> +} *syscall_group_desc_p;
> +DEF_VEC_P(syscall_group_desc_p);
> +
>  /* Structure that represents syscalls information.  */
>  struct syscalls_info
>  {
> @@ -99,6 +126,10 @@ struct syscalls_info
>  
>    VEC(syscall_desc_p) *syscalls;
>  
> +  /* The syscall groups.  */
> +
> +  VEC(syscall_group_desc_p) *groups;
> +
>    /* Variable that will hold the last known data-directory.  This is
>       useful to know whether we should re-read the XML info for the
>       target.  */
> @@ -126,11 +157,21 @@ syscalls_info_free_syscalls_desc (struct syscall_desc *sd)
>    xfree (sd->name);
>  }
>  
> +/* Free syscall_group_desc members but not the structure itself.  */
> +
> +static void
> +syscalls_info_free_syscall_group_desc (struct syscall_group_desc *sd)
> +{
> +  VEC_free (syscall_desc_p, sd->syscalls);
> +  xfree (sd->name);
> +}
> +
>  static void
>  free_syscalls_info (void *arg)
>  {
>    struct syscalls_info *syscalls_info = arg;
>    struct syscall_desc *sysdesc;
> +  struct syscall_group_desc *groupdesc;
>    int i;
>  
>    xfree (syscalls_info->my_gdb_datadir);
> @@ -143,6 +184,15 @@ free_syscalls_info (void *arg)
>  	syscalls_info_free_syscalls_desc (sysdesc);
>        VEC_free (syscall_desc_p, syscalls_info->syscalls);
>      }
> +  if (syscalls_info->groups != NULL)
> +    {
> +      for (i = 0;
> +	   VEC_iterate (syscall_group_desc_p,
> +			syscalls_info->groups, i, groupdesc);
> +	   i++)
> +	syscalls_info_free_syscall_group_desc (groupdesc);
> +      VEC_free (syscall_group_desc_p, syscalls_info->groups);
> +    }
>  
>    xfree (syscalls_info);
>  }
> @@ -153,16 +203,71 @@ make_cleanup_free_syscalls_info (struct syscalls_info *syscalls_info)
>    return make_cleanup (free_syscalls_info, syscalls_info);
>  }
>  
> +/* Create a new syscall group.  Return pointer to the
> +   syscall_group_desc structure that represents the new group.  */
> +
> +static struct syscall_group_desc *
> +syscall_group_create_syscall_group_desc (struct syscalls_info *syscalls_info,
> +					 const char *group)
> +{
> +  struct syscall_group_desc *groupdesc = XCNEW (struct syscall_group_desc);
> +
> +  groupdesc->name = xstrdup (group);
> +
> +  VEC_safe_push (syscall_group_desc_p, syscalls_info->groups, groupdesc);
> +
> +  return groupdesc;
> +}
> +
> +/* Add a syscall to the group.  If group doesn't exist, create
> +   it.  */

Comments like this always look weird to me.
There's PLENTY of space on the previous line so "it"
doesn't have to appear by itself.

> +
> +static void
> +syscall_group_add_syscall (struct syscalls_info *syscalls_info,
> +			   struct syscall_desc *syscall,
> +			   const char *group)
> +{
> +  struct syscall_group_desc *groupdesc;
> +  int i;
> +
> +  /* Search for an existing group.  */
> +  for (i = 0;
> +       VEC_iterate (syscall_group_desc_p,
> +		    syscalls_info->groups, i, groupdesc);
> +       i++)
> +    if (strcmp (groupdesc->name, group) == 0)
> +      break;
> +
> +  if (groupdesc == NULL)
> +    {
> +      /* No group was found with this name.  We must create a new
> +	 one.  */
> +      groupdesc = syscall_group_create_syscall_group_desc (syscalls_info,
> +							   group);
> +    }
> +
> +  VEC_safe_push (syscall_desc_p, groupdesc->syscalls, syscall);
> +}
> +
>  static void
>  syscall_create_syscall_desc (struct syscalls_info *syscalls_info,
> -                             const char *name, int number)
> +                             const char *name, int number,
> +			     char *groups)
>  {
>    struct syscall_desc *sysdesc = XCNEW (struct syscall_desc);
> +  char *group;
>  
>    sysdesc->name = xstrdup (name);
>    sysdesc->number = number;
>  
>    VEC_safe_push (syscall_desc_p, syscalls_info->syscalls, sysdesc);
> +
> +  /*  Add syscall to its groups.  */
> +  if (groups != NULL)
> +    for (group = strtok (groups, ",");
> +	 group != NULL;
> +	 group = strtok (NULL, ","))
> +      syscall_group_add_syscall (syscalls_info, sysdesc, group);

Enclose for() loop in {} braces.

  if (...)
    {
      ...
    }

>  }
>  
>  /* Handle the start of a <syscall> element.  */
> @@ -177,6 +282,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
>    /* syscall info.  */
>    char *name = NULL;
>    int number = 0;
> +  char *groups = NULL;
>  
>    len = VEC_length (gdb_xml_value_s, attributes);
>  
> @@ -186,13 +292,16 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
>          name = attrs[i].value;
>        else if (strcmp (attrs[i].name, "number") == 0)
>          number = * (ULONGEST *) attrs[i].value;
> +      else if (strcmp (attrs[i].name, "groups") == 0)
> +        groups = attrs[i].value;
>        else
>          internal_error (__FILE__, __LINE__,
>                          _("Unknown attribute name '%s'."), attrs[i].name);
>      }
>  
>    gdb_assert (name);
> -  syscall_create_syscall_desc (data->syscalls_info, name, number);
> +
> +  syscall_create_syscall_desc (data->syscalls_info, name, number, groups);
>  }
>  
>  
> @@ -200,6 +309,7 @@ syscall_start_syscall (struct gdb_xml_parser *parser,
>  static const struct gdb_xml_attribute syscall_attr[] = {
>    { "number", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
>    { "name", GDB_XML_AF_NONE, NULL, NULL },
> +  { "groups", GDB_XML_AF_OPTIONAL, NULL, NULL },
>    { NULL, GDB_XML_AF_NONE, NULL, NULL }
>  };
>  
> @@ -321,6 +431,33 @@ init_syscalls_info (struct gdbarch *gdbarch)
>    set_gdbarch_syscalls_info (gdbarch, syscalls_info);
>  }
>  
> +/* Search for a syscall group by its name.  Return syscall_group_desc
> +   structure for the group if found or NULL otherwise.  */
> +
> +static struct syscall_group_desc *
> +syscall_group_get_group_by_name (const struct syscalls_info *syscalls_info,
> +				 const char *group)
> +{
> +  struct syscall_group_desc *groupdesc;
> +  int i;
> +
> +  if (syscalls_info == NULL)
> +    return NULL;
> +
> +  if (group == NULL)
> +    return NULL;
> +
> +   /* Search for existing group.  */
> +  for (i = 0;
> +       VEC_iterate (syscall_group_desc_p,
> +		    syscalls_info->groups, i, groupdesc);
> +       i++)
> +    if (strcmp (groupdesc->name, group) == 0)
> +      return groupdesc;

Enclose if () in {} braces.

  for (...)
    {
      if (...)
	...;
    }

> +
> +  return NULL;
> +}
> +
>  static int
>  xml_get_syscall_number (struct gdbarch *gdbarch,
>                          const char *syscall_name)
> @@ -388,6 +525,75 @@ xml_list_of_syscalls (struct gdbarch *gdbarch)
>    return names;
>  }
>  
> +/*  Iterate over the syscall_group_desc element to return a list of
> +    syscalls that are part of the given group, terminated by an empty
> +    element.  If the syscall group doesn't exist, return NULL.  */

Extra space of indentation.

> +
> +static struct syscall *
> +xml_list_syscalls_by_group (struct gdbarch *gdbarch,
> +			    const char *group)
> +{
> +  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
> +  struct syscall_group_desc *groupdesc;
> +  struct syscall_desc *sysdesc;
> +  struct syscall *syscalls = NULL;
> +  int nsyscalls;
> +  int i;
> +
> +  if (syscalls_info == NULL)
> +    return NULL;
> +
> +  groupdesc = syscall_group_get_group_by_name (syscalls_info, group);
> +  if (groupdesc == NULL)
> +    return NULL;
> +
> +  nsyscalls = VEC_length (syscall_desc_p, groupdesc->syscalls);
> +  syscalls = xmalloc ((nsyscalls + 1) * sizeof (struct syscall));
> +
> +  for (i = 0;
> +       VEC_iterate (syscall_desc_p, groupdesc->syscalls, i, sysdesc);
> +       i++)
> +    {
> +      syscalls[i].name = sysdesc->name;
> +      syscalls[i].number = sysdesc->number;
> +    }
> +
> +  /* Add final element marker.  */
> +  syscalls[i].name = NULL;
> +  syscalls[i].number = 0;
> +
> +  return syscalls;
> +}
> +
> +/* Return the list of syscall groups.  If no syscall group is
> +   available, return NULL.  */

The comment doesn't match what the function returns if syscalls_info
is non-NULL but syscalls_info->groups is NULL.  Can that happen?

> +
> +static const char **
> +xml_list_of_groups (struct gdbarch *gdbarch)
> +{
> +  struct syscalls_info *syscalls_info = gdbarch_syscalls_info (gdbarch);
> +  struct syscall_group_desc *groupdesc;
> +  const char **names = NULL;
> +  int i;
> +  int ngroups;
> +
> +  if (syscalls_info == NULL)
> +    return NULL;
> +
> +  ngroups = VEC_length (syscall_group_desc_p, syscalls_info->groups);
> +  names = xmalloc ((ngroups + 1) * sizeof (char *));
> +
> +  for (i = 0;
> +       VEC_iterate (syscall_group_desc_p,
> +		    syscalls_info->groups, i, groupdesc);
> +       i++)
> +    names[i] = groupdesc->name;
> +
> +  names[i] = NULL;
> +
> +  return names;
> +}
> +
>  void
>  set_xml_syscall_file_name (struct gdbarch *gdbarch, const char *name)
>  {
> @@ -422,4 +628,24 @@ get_syscall_names (struct gdbarch *gdbarch)
>    return xml_list_of_syscalls (gdbarch);
>  }
>  
> +/* See comment in xml-syscall.h.  */
> +
> +struct syscall *
> +get_syscalls_by_group (struct gdbarch *gdbarch, const char *group)
> +{
> +  init_syscalls_info (gdbarch);
> +
> +  return xml_list_syscalls_by_group (gdbarch, group);
> +}
> +
> +/* See comment in xml-syscall.h.  */
> +
> +const char **
> +get_syscall_group_names (struct gdbarch *gdbarch)
> +{
> +  init_syscalls_info (gdbarch);
> +
> +  return xml_list_of_groups (gdbarch);
> +}
> +
>  #endif /* ! HAVE_LIBEXPAT */
> diff --git a/gdb/xml-syscall.h b/gdb/xml-syscall.h
> index 42b9dea..9ba4e93 100644
> --- a/gdb/xml-syscall.h
> +++ b/gdb/xml-syscall.h
> @@ -50,4 +50,19 @@ void get_syscall_by_name (struct gdbarch *gdbarch,
>  
>  const char **get_syscall_names (struct gdbarch *gdbarch);
>  
> +/* Function used to retrieve the list of syscalls of a given group in
> +   the system.  Return a list of syscalls that are element of the
> +   group, terminated by an empty element. The list is malloc'ed
> +   and must be freed by the caller.  If group doesn't exist, return
> +   NULL.  */
> +
> +struct syscall *get_syscalls_by_group (struct gdbarch *gdbarch,
> +				       const char *group);
> +
> +/* Function used to retrieve the list of syscall groups in the system.
> +   Return an array of strings terminated by a NULL element.  The list
> +   must be freed by the caller.  */

The function can return NULL but the comment doesn't mention this.

> +
> +const char **get_syscall_group_names (struct gdbarch *gdbarch);
> +
>  #endif /* XML_SYSCALL_H */
  

Doug Evans <xdje42@gmail.com> writes:

> Hi.
> I didn't make sure that I'm not repeating something Sergio mentioned.
>
Doug,

Thanks for your review.

Unfortunately, right now I have some issues with my copyright
assignment status and I should solve it before I can provide the changes
you mentioned and put this upstream.  It took me some time to answer
because of that, but I think it is gonna take a few more weeks until
everything is in place.

Let me put these patches on hold for a while and as soon as everything
is set, I'll ping you and the list with the updated version to include
the suggestions you and Sergio made.

Thanks!
  

On Wed, Jan 28, 2015 at 12:42 PM, Gabriel Krisman Bertazi
<gabriel@krisman.be> wrote:
> Doug Evans <xdje42@gmail.com> writes:
>
>> Hi.
>> I didn't make sure that I'm not repeating something Sergio mentioned.
>>
> Doug,
>
> Thanks for your review.
>
> Unfortunately, right now I have some issues with my copyright
> assignment status and I should solve it before I can provide the changes
> you mentioned and put this upstream.  It took me some time to answer
> because of that, but I think it is gonna take a few more weeks until
> everything is in place.
>
> Let me put these patches on hold for a while and as soon as everything
> is set, I'll ping you and the list with the updated version to include
> the suggestions you and Sergio made.
>
> Thanks!

Ok, no worries.