1. Patchew
  2. QEMU
  3. [RFC PATCH 00/24] plugins: Allow to read registers
  4. View patch

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

Akihiko Odaki posted 24 patches 2 years, 6 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Richard Henderson <richard.henderson@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Brian Cain <bcain@quicinc.com>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Laurent Vivier <laurent@vivier.eu>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Nicholas Piggin <npiggin@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
There is a newer version of this series
[RFC PATCH 12/24] gdbstub: Simplify XML lookup
Posted by Akihiko Odaki 2 years, 6 months ago 
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;
     }
-    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)
-- 
2.41.0
Re: [RFC PATCH 12/24] gdbstub: Simplify XML lookup
Posted by Alex Bennée 2 years, 5 months ago 
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
Re: [RFC PATCH 12/24] gdbstub: Simplify XML lookup
Posted by Akihiko Odaki 2 years, 5 months ago 
On 2023/08/14 22:27, Alex Bennée wrote:
> 
> 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.

I saw you did that yourself. Nevertheless I included my own 
implementation for the suggestion in v3. It uses 
g_markup_printf_escaped() for extra caution and better readability (i.e. 
the xi:include tags are written in a format: <xi:include href=\"%s\"/>).

> 
> 
>> -    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)
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.