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

Thiago Jung Bauermann
2025-07-08T04:05:00Z

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