Re: [PATCH v2 07/47] gdb, gdbserver, gdbsupport: add 'device' tag to XML target description

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> writes:

> diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
> index 436c493d4f914eb71a80bcbc08dc2ddd38c72027..61cba42e9eee2ec4cc66e14a120c5bef909e48b0 100644
> --- a/gdb/xml-tdesc.c
> +++ b/gdb/xml-tdesc.c
> @@ -137,6 +137,65 @@ tdesc_end_compatible (struct gdb_xml_parser *parser,
>    tdesc_add_compatible (data->tdesc, arch);
>  }
>  
> +/* Handle the end of a <device> element and its value.  */

s/the end/the start/

> +static void
> +tdesc_start_device (struct gdb_xml_parser *parser,
> +		    const gdb_xml_element *element,
> +		    void *user_data, std::vector<gdb_xml_value> &attributes)
> +{

<snip>

> +void
> +print_xml_feature::visit (const tdesc_device *device)
> +{
> +  if (device == nullptr)
> +    return;
> +
> +  std::string tmp = "<device";
> +  if (device->vendor_id.has_value ())
> +    string_appendf (tmp, " vendor-id=\"0x%04" PRIx32 "\"",
> +		    *device->vendor_id);
> +
> +  if (device->target_id.has_value ())
> +    string_appendf (tmp, " target-id=\"0x%04" PRIx32 "\"",
> +		    *device->target_id);
> +
> +  string_appendf (tmp, " family=\"%s\"", device->family.c_str ());
> +  string_appendf (tmp, " model=\"%s\"", device->model.c_str ());

For all the string attributes in device: isn't it better to check
whether the string is empty and only emit the attribute if it isn't?

> +  if (device->stepping.has_value ())
> +    string_appendf (tmp, " stepping=\"%d\"", *device->stepping);
> +
> +  string_appendf (tmp, " name=\"%s\"", device->name.c_str ());
> +  string_appendf (tmp, " pci-slot=\"%s\"", device->pci_slot.c_str ());
> +  string_appendf (tmp, " uuid=\"%s\"", device->uuid.c_str ());
> +
> +  if (device->total_cores.has_value ())
> +    string_appendf (tmp, " total-cores=\"%ld\"", *device->total_cores);
> +
> +  if (device->total_threads.has_value ())
> +    string_appendf (tmp, " total-threads=\"%ld\"", *device->total_threads);
> +
> +  if (device->subdevice_id.has_value ())
> +    string_appendf (tmp, " subdevice-id=\"%d\"", *device->subdevice_id);
> +
> +  string_appendf (tmp, "/>");
> +  add_line (tmp);
> +}
> +
>  /* See gdbsupport/tdesc.h.  */
>  
>  void
> diff --git a/gdbsupport/tdesc.h b/gdbsupport/tdesc.h
> index c9e7603369cbf986187908c2d1f8fbe35a0dc02d..29de4d231f16667c8297bcbe403f683c30a28762 100644
> --- a/gdbsupport/tdesc.h
> +++ b/gdbsupport/tdesc.h
> @@ -26,6 +26,7 @@ struct tdesc_type_builtin;
>  struct tdesc_type_vector;
>  struct tdesc_type_with_fields;
>  struct tdesc_reg;
> +struct tdesc_device;
>  struct target_desc;
>  
>  /* The interface to visit different elements of target description.  */
> @@ -56,6 +57,9 @@ class tdesc_element_visitor
>  
>    virtual void visit (const tdesc_reg *e)
>    {}
> +
> +  virtual void visit (const tdesc_device *e)
> +  {}
>  };
>  
>  class tdesc_element
> @@ -315,6 +319,84 @@ struct tdesc_feature : tdesc_element
>  
>  typedef std::unique_ptr<tdesc_feature> tdesc_feature_up;
>  
> +/* The device information in a target description.  */
> +
> +struct tdesc_device : tdesc_element
> +{
> +  tdesc_device ()
> +  {
> +    family = "-";
> +    model = "-";
> +    name = "-";
> +    pci_slot = "-";
> +    uuid = "-";

Why aren't the strings left empty?

> +  }

-- 
Thiago