Re: [RFC PATCH 12/24] gdbstub: Simplify XML lookup

[Alex Bennée]

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> Now we know all instances of GDBFeature that is used in CPU so we can
> traverse them to find XML. This removes the need for a CPU-specific
> lookup function for dynamic XMLs.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  gdbstub/gdbstub.c | 28 +++++++++-------------------
>  1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 182efe7e0f..e5bb2c89ba 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -354,8 +354,7 @@ static const char *get_feature_xml(const char *p, const 
> char **newp,
>                                     GDBProcess *process)
>  {
>      size_t len;
> -    int i;
> -    const char *name;
> +    GDBRegisterState *r;
>      CPUState *cpu = gdb_get_first_cpu_in_process(process);
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>  
> @@ -364,15 +363,12 @@ static const char *get_feature_xml(const char *p, const 
> char **newp,
>          len++;
>      *newp = p + len;
>  
> -    name = NULL;
>      if (strncmp(p, "target.xml", len) == 0) {
>          char *buf = process->target_xml;
>          const size_t buf_sz = sizeof(process->target_xml);
>  
>          /* Generate the XML description for this CPU.  */
>          if (!buf[0]) {
> -            GDBRegisterState *r;
> -
>              pstrcat(buf, buf_sz,
>                      "<?xml version=\"1.0\"?>"
>                      "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> @@ -389,28 +385,22 @@ static const char *get_feature_xml(const char *p, const 
> char **newp,
>              pstrcat(buf, buf_sz, "\"/>");
>              for (r = cpu->gdb_regs; r; r = r->next) {
>                  pstrcat(buf, buf_sz, "<xi:include href=\"");
> -                pstrcat(buf, buf_sz, r->feature->xml);
> +                pstrcat(buf, buf_sz, r->feature->xmlname);
>                  pstrcat(buf, buf_sz, "\"/>");
>              }
>              pstrcat(buf, buf_sz, "</target>");
>          }
>          return buf;
>      }

It would be nice to modernise this code before adding to it. The static
target_xml buffer and use of pstrcat could be replaced by GString code
that is less sketchy.


> -    if (cc->gdb_get_dynamic_xml) {
> -        char *xmlname = g_strndup(p, len);
> -        const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
> -
> -        g_free(xmlname);
> -        if (xml) {
> -            return xml;
> -        }
> +    if (strncmp(p, cc->gdb_core_feature->xmlname, len) == 0) {
> +        return cc->gdb_core_feature->xml;
>      }
> -    for (i = 0; ; i++) {
> -        name = gdb_features[i].xmlname;
> -        if (!name || (strncmp(name, p, len) == 0 && strlen(name) == len))
> -            break;
> +    for (r = cpu->gdb_regs; r; r = r->next) {
> +        if (strncmp(p, r->feature->xmlname, len) == 0) {
> +            return r->feature->xml;
> +        }
>      }
> -    return name ? gdb_features[i].xml : NULL;
> +    return NULL;
>  }
>  
>  const GDBFeature *gdb_find_static_feature(const char *xmlname)


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro