qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] misc: Replace alloca() by g_malloc()
@ 2021-05-05 17:00 Philippe Mathieu-Daudé
  2021-05-05 17:00 ` [PATCH 1/5] bsd-user/syscall: Replace alloca() by g_new() Philippe Mathieu-Daudé
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-05 17:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kyle Evans, Greg Kurz, Alex Bennée, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Warner Losh, David Gibson

The ALLOCA(3) man-page mentions its "use is discouraged".
Replace few calls by a g_malloc()/g_new() ones.

Last call site is linux-user/.

Philippe Mathieu-Daudé (5):
  bsd-user/syscall: Replace alloca() by g_new()
  gdbstub: Constify GdbCmdParseEntry
  gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer
  gdbstub: Replace alloca() by g_new()
  target/ppc/kvm: Replace alloca() by g_malloc()

 bsd-user/syscall.c |  3 +--
 gdbstub.c          | 24 +++++++++++++-----------
 target/ppc/kvm.c   | 10 +++++-----
 3 files changed, 19 insertions(+), 18 deletions(-)

-- 
2.26.3




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] bsd-user/syscall: Replace alloca() by g_new()
  2021-05-05 17:00 [PATCH 0/5] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
@ 2021-05-05 17:00 ` Philippe Mathieu-Daudé
  2021-05-05 17:00 ` [PATCH 2/5] gdbstub: Constify GdbCmdParseEntry Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-05 17:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kyle Evans, Greg Kurz, Alex Bennée, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Warner Losh, David Gibson

The ALLOCA(3) man-page mentions its "use is discouraged".

Replace it by a g_new() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 bsd-user/syscall.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
index 4abff796c76..dbee0385ceb 100644
--- a/bsd-user/syscall.c
+++ b/bsd-user/syscall.c
@@ -355,9 +355,8 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_FREEBSD_NR_writev:
         {
             int count = arg3;
-            struct iovec *vec;
+            g_autofree struct iovec *vec = g_new(struct iovec, count);
 
-            vec = alloca(count * sizeof(struct iovec));
             if (lock_iovec(VERIFY_READ, vec, arg2, count, 1) < 0)
                 goto efault;
             ret = get_errno(writev(arg1, vec, count));
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] gdbstub: Constify GdbCmdParseEntry
  2021-05-05 17:00 [PATCH 0/5] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
  2021-05-05 17:00 ` [PATCH 1/5] bsd-user/syscall: Replace alloca() by g_new() Philippe Mathieu-Daudé
@ 2021-05-05 17:00 ` Philippe Mathieu-Daudé
  2021-05-06 11:50   ` Alex Bennée
  2021-05-05 17:00 ` [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-05 17:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kyle Evans, Greg Kurz, Alex Bennée, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Warner Losh, David Gibson

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 gdbstub.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 054665e93ea..6f663cbd8dd 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1982,7 +1982,7 @@ static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
     exit(0);
 }
 
-static GdbCmdParseEntry gdb_v_commands_table[] = {
+static const GdbCmdParseEntry gdb_v_commands_table[] = {
     /* Order is important if has same prefix */
     {
         .handler = handle_v_cont_query,
@@ -2325,7 +2325,7 @@ static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
 }
 #endif
 
-static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
+static const GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
     /* Order is important if has same prefix */
     {
         .handler = handle_query_qemu_sstepbits,
@@ -2343,7 +2343,7 @@ static GdbCmdParseEntry gdb_gen_query_set_common_table[] = {
     },
 };
 
-static GdbCmdParseEntry gdb_gen_query_table[] = {
+static const GdbCmdParseEntry gdb_gen_query_table[] = {
     {
         .handler = handle_query_curr_tid,
         .cmd = "C",
@@ -2421,7 +2421,7 @@ static GdbCmdParseEntry gdb_gen_query_table[] = {
 #endif
 };
 
-static GdbCmdParseEntry gdb_gen_set_table[] = {
+static const GdbCmdParseEntry gdb_gen_set_table[] = {
     /* Order is important if has same prefix */
     {
         .handler = handle_set_qemu_sstep,
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer
  2021-05-05 17:00 [PATCH 0/5] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
  2021-05-05 17:00 ` [PATCH 1/5] bsd-user/syscall: Replace alloca() by g_new() Philippe Mathieu-Daudé
  2021-05-05 17:00 ` [PATCH 2/5] gdbstub: Constify GdbCmdParseEntry Philippe Mathieu-Daudé
@ 2021-05-05 17:00 ` Philippe Mathieu-Daudé
  2021-05-06 12:01   ` Alex Bennée
  2021-05-05 17:00 ` [PATCH 4/5] gdbstub: Replace alloca() by g_new() Philippe Mathieu-Daudé
  2021-05-05 17:00 ` [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
  4 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-05 17:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kyle Evans, Greg Kurz, Alex Bennée, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Warner Losh, David Gibson

GdbCmdParseEntry should have enough room with 20 chars for the command
string, and 8 for the schema. Add the GDB_CMD_PARSE_ENTRY_CMD_SIZE and
GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE definitions.

Do not use pointer to string of unknown length, but array of fixed
size. Having constant size will help use to remove the alloca() call
in process_string_cmd() in the next commit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 gdbstub.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 6f663cbd8dd..0d5569ee539 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1457,11 +1457,13 @@ typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
  * '.' -> Skip 1 char unless reached "\0"
  * Any other value is treated as the delimiter value itself
  */
+#define GDB_CMD_PARSE_ENTRY_CMD_SIZE    20
+#define GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE 8
 typedef struct GdbCmdParseEntry {
     GdbCmdHandler handler;
-    const char *cmd;
+    const char cmd[GDB_CMD_PARSE_ENTRY_CMD_SIZE];
     bool cmd_startswith;
-    const char *schema;
+    const char schema[GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE];
 } GdbCmdParseEntry;
 
 static inline int startswith(const char *string, const char *pattern)
@@ -1481,14 +1483,14 @@ static int process_string_cmd(void *user_ctx, const char *data,
 
     for (i = 0; i < num_cmds; i++) {
         const GdbCmdParseEntry *cmd = &cmds[i];
-        g_assert(cmd->handler && cmd->cmd);
+        g_assert(cmd->handler && *cmd->cmd);
 
         if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
             (!cmd->cmd_startswith && strcmp(cmd->cmd, data))) {
             continue;
         }
 
-        if (cmd->schema) {
+        if (*cmd->schema) {
             schema_len = strlen(cmd->schema);
             if (schema_len % 2) {
                 return -2;
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] gdbstub: Replace alloca() by g_new()
  2021-05-05 17:00 [PATCH 0/5] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-05-05 17:00 ` [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer Philippe Mathieu-Daudé
@ 2021-05-05 17:00 ` Philippe Mathieu-Daudé
  2021-05-06 16:07   ` [ALT PATCH] gdbstub: Replace GdbCmdContext with plain g_array() Alex Bennée
  2021-05-05 17:00 ` [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
  4 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-05 17:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kyle Evans, Greg Kurz, Alex Bennée, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Warner Losh, David Gibson

The ALLOCA(3) man-page mentions its "use is discouraged".

Replace it by a g_new() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 gdbstub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 0d5569ee539..72b4be89c7b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1475,7 +1475,9 @@ static int process_string_cmd(void *user_ctx, const char *data,
                               const GdbCmdParseEntry *cmds, int num_cmds)
 {
     int i, schema_len, max_num_params = 0;
-    GdbCmdContext gdb_ctx;
+    g_autofree GdbCmdVariant *params = g_new(GdbCmdVariant,
+                                        GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE / 2);
+    GdbCmdContext gdb_ctx = { .params = params };
 
     if (!cmds) {
         return -1;
@@ -1499,8 +1501,6 @@ static int process_string_cmd(void *user_ctx, const char *data,
             max_num_params = schema_len / 2;
         }
 
-        gdb_ctx.params =
-            (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params) * max_num_params);
         memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
 
         if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc()
  2021-05-05 17:00 [PATCH 0/5] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-05-05 17:00 ` [PATCH 4/5] gdbstub: Replace alloca() by g_new() Philippe Mathieu-Daudé
@ 2021-05-05 17:00 ` Philippe Mathieu-Daudé
  2021-05-06  2:18   ` David Gibson
  2021-05-06  8:41   ` Greg Kurz
  4 siblings, 2 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-05 17:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: open list:Overall KVM CPUs, Kyle Evans, Greg Kurz,
	Alex Bennée, open list:PowerPC TCG CPUs, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Warner Losh, David Gibson

The ALLOCA(3) man-page mentions its "use is discouraged".

Replace it by a g_malloc() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/ppc/kvm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 104a308abb5..ae62daddf7d 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2698,11 +2698,11 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
 int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
                            uint16_t n_valid, uint16_t n_invalid, Error **errp)
 {
-    struct kvm_get_htab_header *buf;
-    size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
+    size_t chunksize = sizeof(struct kvm_get_htab_header)
+                       + n_valid * HASH_PTE_SIZE_64;
     ssize_t rc;
+    g_autofree struct kvm_get_htab_header *buf = g_malloc(chunksize);
 
-    buf = alloca(chunksize);
     buf->index = index;
     buf->n_valid = n_valid;
     buf->n_invalid = n_invalid;
@@ -2741,10 +2741,10 @@ void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)
     i = 0;
     while (i < n) {
         struct kvm_get_htab_header *hdr;
+        char buf[sizeof(*hdr) + HPTES_PER_GROUP * HASH_PTE_SIZE_64];
         int m = n < HPTES_PER_GROUP ? n : HPTES_PER_GROUP;
-        char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64];
 
-        rc = read(fd, buf, sizeof(buf));
+        rc = read(fd, buf, sizeof(*hdr) + m * HASH_PTE_SIZE_64);
         if (rc < 0) {
             hw_error("kvmppc_read_hptes: Unable to read HPTEs");
         }
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc()
  2021-05-05 17:00 ` [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
@ 2021-05-06  2:18   ` David Gibson
  2021-05-06  8:41   ` Greg Kurz
  1 sibling, 0 replies; 14+ messages in thread
From: David Gibson @ 2021-05-06  2:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: open list:Overall KVM CPUs, Kyle Evans, Greg Kurz, qemu-devel,
	open list:PowerPC TCG CPUs, Paolo Bonzini, Alex Bennée,
	Warner Losh

[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]

On Wed, May 05, 2021 at 07:00:55PM +0200, Philippe Mathieu-Daudé wrote:
> The ALLOCA(3) man-page mentions its "use is discouraged".
> 
> Replace it by a g_malloc() call.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  target/ppc/kvm.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb5..ae62daddf7d 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2698,11 +2698,11 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
>  int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
>                             uint16_t n_valid, uint16_t n_invalid, Error **errp)
>  {
> -    struct kvm_get_htab_header *buf;
> -    size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
> +    size_t chunksize = sizeof(struct kvm_get_htab_header)
> +                       + n_valid * HASH_PTE_SIZE_64;
>      ssize_t rc;
> +    g_autofree struct kvm_get_htab_header *buf = g_malloc(chunksize);
>  
> -    buf = alloca(chunksize);
>      buf->index = index;
>      buf->n_valid = n_valid;
>      buf->n_invalid = n_invalid;
> @@ -2741,10 +2741,10 @@ void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)
>      i = 0;
>      while (i < n) {
>          struct kvm_get_htab_header *hdr;
> +        char buf[sizeof(*hdr) + HPTES_PER_GROUP * HASH_PTE_SIZE_64];
>          int m = n < HPTES_PER_GROUP ? n : HPTES_PER_GROUP;
> -        char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64];
>  
> -        rc = read(fd, buf, sizeof(buf));
> +        rc = read(fd, buf, sizeof(*hdr) + m * HASH_PTE_SIZE_64);
>          if (rc < 0) {
>              hw_error("kvmppc_read_hptes: Unable to read HPTEs");
>          }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc()
  2021-05-05 17:00 ` [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
  2021-05-06  2:18   ` David Gibson
@ 2021-05-06  8:41   ` Greg Kurz
  2021-05-06 13:09     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 14+ messages in thread
From: Greg Kurz @ 2021-05-06  8:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: open list:Overall KVM CPUs, Kyle Evans, qemu-devel,
	open list:PowerPC TCG CPUs, Paolo Bonzini, Alex Bennée,
	Warner Losh, David Gibson

On Wed,  5 May 2021 19:00:55 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> The ALLOCA(3) man-page mentions its "use is discouraged".
> 
> Replace it by a g_malloc() call.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  target/ppc/kvm.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 104a308abb5..ae62daddf7d 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -2698,11 +2698,11 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
>  int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
>                             uint16_t n_valid, uint16_t n_invalid, Error **errp)
>  {
> -    struct kvm_get_htab_header *buf;
> -    size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
> +    size_t chunksize = sizeof(struct kvm_get_htab_header)

It is a bit unfortunate to introduce a new dependency on the struct type.

What about the following ?

-    struct kvm_get_htab_header *buf;
+    g_autofree struct kvm_get_htab_header *buf = NULL;
     size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
     ssize_t rc;
 
-    buf = alloca(chunksize);
+    buf = g_malloc(chunksize);


    g_autofree struct kvm_get_htab_header *buf = NULL;
    size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;

> +                       + n_valid * HASH_PTE_SIZE_64;
>      ssize_t rc;
> +    g_autofree struct kvm_get_htab_header *buf = g_malloc(chunksize);
>  
> -    buf = alloca(chunksize);
>      buf->index = index;
>      buf->n_valid = n_valid;
>      buf->n_invalid = n_invalid;
> @@ -2741,10 +2741,10 @@ void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)
>      i = 0;
>      while (i < n) {
>          struct kvm_get_htab_header *hdr;
> +        char buf[sizeof(*hdr) + HPTES_PER_GROUP * HASH_PTE_SIZE_64];
>          int m = n < HPTES_PER_GROUP ? n : HPTES_PER_GROUP;
> -        char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64];
>  
> -        rc = read(fd, buf, sizeof(buf));
> +        rc = read(fd, buf, sizeof(*hdr) + m * HASH_PTE_SIZE_64);
>          if (rc < 0) {
>              hw_error("kvmppc_read_hptes: Unable to read HPTEs");
>          }



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] gdbstub: Constify GdbCmdParseEntry
  2021-05-05 17:00 ` [PATCH 2/5] gdbstub: Constify GdbCmdParseEntry Philippe Mathieu-Daudé
@ 2021-05-06 11:50   ` Alex Bennée
  0 siblings, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2021-05-06 11:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Kyle Evans, Greg Kurz, qemu-devel, Paolo Bonzini, Warner Losh,
	David Gibson


Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer
  2021-05-05 17:00 ` [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer Philippe Mathieu-Daudé
@ 2021-05-06 12:01   ` Alex Bennée
  2021-05-06 12:39     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Alex Bennée @ 2021-05-06 12:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Kyle Evans, Greg Kurz, qemu-devel, Paolo Bonzini, Warner Losh,
	David Gibson


Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> GdbCmdParseEntry should have enough room with 20 chars for the command
> string, and 8 for the schema. Add the GDB_CMD_PARSE_ENTRY_CMD_SIZE and
> GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE definitions.
>
> Do not use pointer to string of unknown length, but array of fixed
> size. Having constant size will help use to remove the alloca() call
> in process_string_cmd() in the next commit.

I don't understand how this helps. The alloca is being used for an array
of GdbCmdVariant so why do we want to enlarge the size of the parse
table entries?

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  gdbstub.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 6f663cbd8dd..0d5569ee539 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1457,11 +1457,13 @@ typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
>   * '.' -> Skip 1 char unless reached "\0"
>   * Any other value is treated as the delimiter value itself
>   */
> +#define GDB_CMD_PARSE_ENTRY_CMD_SIZE    20
> +#define GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE 8
>  typedef struct GdbCmdParseEntry {
>      GdbCmdHandler handler;
> -    const char *cmd;
> +    const char cmd[GDB_CMD_PARSE_ENTRY_CMD_SIZE];
>      bool cmd_startswith;
> -    const char *schema;
> +    const char schema[GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE];
>  } GdbCmdParseEntry;
>  
>  static inline int startswith(const char *string, const char *pattern)
> @@ -1481,14 +1483,14 @@ static int process_string_cmd(void *user_ctx, const char *data,
>  
>      for (i = 0; i < num_cmds; i++) {
>          const GdbCmdParseEntry *cmd = &cmds[i];
> -        g_assert(cmd->handler && cmd->cmd);
> +        g_assert(cmd->handler && *cmd->cmd);
>  
>          if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
>              (!cmd->cmd_startswith && strcmp(cmd->cmd, data))) {
>              continue;
>          }
>  
> -        if (cmd->schema) {
> +        if (*cmd->schema) {
>              schema_len = strlen(cmd->schema);
>              if (schema_len % 2) {
>                  return -2;


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer
  2021-05-06 12:01   ` Alex Bennée
@ 2021-05-06 12:39     ` Philippe Mathieu-Daudé
  2021-05-06 15:15       ` Alex Bennée
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 12:39 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Kyle Evans, Greg Kurz, qemu-devel, Paolo Bonzini, Warner Losh,
	David Gibson

On 5/6/21 2:01 PM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> GdbCmdParseEntry should have enough room with 20 chars for the command
>> string, and 8 for the schema. Add the GDB_CMD_PARSE_ENTRY_CMD_SIZE and
>> GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE definitions.
>>
>> Do not use pointer to string of unknown length, but array of fixed
>> size. Having constant size will help use to remove the alloca() call
>> in process_string_cmd() in the next commit.
> 
> I don't understand how this helps. The alloca is being used for an array
> of GdbCmdVariant so why do we want to enlarge the size of the parse
> table entries?

This patch is crap indeed. I'll post another one with more sense.

Sorry about this,

Phil.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc()
  2021-05-06  8:41   ` Greg Kurz
@ 2021-05-06 13:09     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-06 13:09 UTC (permalink / raw)
  To: Greg Kurz
  Cc: open list:Overall KVM CPUs, Kyle Evans, qemu-devel,
	open list:PowerPC TCG CPUs, Paolo Bonzini, Alex Bennée,
	Warner Losh, David Gibson

On 5/6/21 10:41 AM, Greg Kurz wrote:
> On Wed,  5 May 2021 19:00:55 +0200
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
>> The ALLOCA(3) man-page mentions its "use is discouraged".
>>
>> Replace it by a g_malloc() call.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  target/ppc/kvm.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 104a308abb5..ae62daddf7d 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -2698,11 +2698,11 @@ int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns)
>>  int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
>>                             uint16_t n_valid, uint16_t n_invalid, Error **errp)
>>  {
>> -    struct kvm_get_htab_header *buf;
>> -    size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
>> +    size_t chunksize = sizeof(struct kvm_get_htab_header)
> 
> It is a bit unfortunate to introduce a new dependency on the struct type.
> 
> What about the following ?
> 
> -    struct kvm_get_htab_header *buf;
> +    g_autofree struct kvm_get_htab_header *buf = NULL;
>      size_t chunksize = sizeof(*buf) + n_valid * HASH_PTE_SIZE_64;
>      ssize_t rc;
>  
> -    buf = alloca(chunksize);
> +    buf = g_malloc(chunksize);

OK.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer
  2021-05-06 12:39     ` Philippe Mathieu-Daudé
@ 2021-05-06 15:15       ` Alex Bennée
  0 siblings, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2021-05-06 15:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Kyle Evans, Greg Kurz, qemu-devel, Paolo Bonzini, Warner Losh,
	David Gibson


Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/6/21 2:01 PM, Alex Bennée wrote:
>> 
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> GdbCmdParseEntry should have enough room with 20 chars for the command
>>> string, and 8 for the schema. Add the GDB_CMD_PARSE_ENTRY_CMD_SIZE and
>>> GDB_CMD_PARSE_ENTRY_SCHEMA_SIZE definitions.
>>>
>>> Do not use pointer to string of unknown length, but array of fixed
>>> size. Having constant size will help use to remove the alloca() call
>>> in process_string_cmd() in the next commit.
>> 
>> I don't understand how this helps. The alloca is being used for an array
>> of GdbCmdVariant so why do we want to enlarge the size of the parse
>> table entries?
>
> This patch is crap indeed. I'll post another one with more sense.

Looking at the logic I wonder it's just better turning params into a
GArray and let glib deal with the sizing for us? It's not like one or
two entries breaks the bank on temporary memory allocation.

>
> Sorry about this,
>
> Phil.


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [ALT PATCH] gdbstub: Replace GdbCmdContext with plain g_array()
  2021-05-05 17:00 ` [PATCH 4/5] gdbstub: Replace alloca() by g_new() Philippe Mathieu-Daudé
@ 2021-05-06 16:07   ` Alex Bennée
  0 siblings, 0 replies; 14+ messages in thread
From: Alex Bennée @ 2021-05-06 16:07 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alex Bennée, Philippe Mathieu-Daudé, f4bug

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Instead of jumping through hoops let glib deal with both tracking the
number of elements and auto freeing the memory once we are done. This
allows is to drop the usage of ALLOCA(3) which the man-page mentions
its "use is discouraged".

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub.c | 314 +++++++++++++++++++++++++-----------------------------
 1 file changed, 146 insertions(+), 168 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 6f663cbd8d..affe5f719f 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1339,6 +1339,8 @@ typedef union GdbCmdVariant {
     } thread_id;
 } GdbCmdVariant;
 
+#define get_param(p, i)    (&g_array_index(p, GdbCmdVariant, i))
+
 static const char *cmd_next_param(const char *param, const char delimiter)
 {
     static const char all_delimiters[] = ",;:=";
@@ -1364,54 +1366,46 @@ static const char *cmd_next_param(const char *param, const char delimiter)
 }
 
 static int cmd_parse_params(const char *data, const char *schema,
-                            GdbCmdVariant *params, int *num_params)
+                            GArray *params)
 {
-    int curr_param;
     const char *curr_schema, *curr_data;
 
-    *num_params = 0;
-
-    if (!schema) {
-        return 0;
-    }
+    g_assert(schema);
+    g_assert(params->len == 0);
 
     curr_schema = schema;
-    curr_param = 0;
     curr_data = data;
     while (curr_schema[0] && curr_schema[1] && *curr_data) {
+        GdbCmdVariant this_param;
+
         switch (curr_schema[0]) {
         case 'l':
             if (qemu_strtoul(curr_data, &curr_data, 16,
-                             &params[curr_param].val_ul)) {
+                             &this_param.val_ul)) {
                 return -EINVAL;
             }
-            curr_param++;
             curr_data = cmd_next_param(curr_data, curr_schema[1]);
             break;
         case 'L':
             if (qemu_strtou64(curr_data, &curr_data, 16,
-                              (uint64_t *)&params[curr_param].val_ull)) {
+                              (uint64_t *)&this_param.val_ull)) {
                 return -EINVAL;
             }
-            curr_param++;
             curr_data = cmd_next_param(curr_data, curr_schema[1]);
             break;
         case 's':
-            params[curr_param].data = curr_data;
-            curr_param++;
+            this_param.data = curr_data;
             curr_data = cmd_next_param(curr_data, curr_schema[1]);
             break;
         case 'o':
-            params[curr_param].opcode = *(uint8_t *)curr_data;
-            curr_param++;
+            this_param.opcode = *(uint8_t *)curr_data;
             curr_data = cmd_next_param(curr_data, curr_schema[1]);
             break;
         case 't':
-            params[curr_param].thread_id.kind =
+            this_param.thread_id.kind =
                 read_thread_id(curr_data, &curr_data,
-                               &params[curr_param].thread_id.pid,
-                               &params[curr_param].thread_id.tid);
-            curr_param++;
+                               &this_param.thread_id.pid,
+                               &this_param.thread_id.tid);
             curr_data = cmd_next_param(curr_data, curr_schema[1]);
             break;
         case '?':
@@ -1420,19 +1414,14 @@ static int cmd_parse_params(const char *data, const char *schema,
         default:
             return -EINVAL;
         }
+        g_array_append_val(params, this_param);
         curr_schema += 2;
     }
 
-    *num_params = curr_param;
     return 0;
 }
 
-typedef struct GdbCmdContext {
-    GdbCmdVariant *params;
-    int num_params;
-} GdbCmdContext;
-
-typedef void (*GdbCmdHandler)(GdbCmdContext *gdb_ctx, void *user_ctx);
+typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx);
 
 /*
  * cmd_startswith -> cmd is compared using startswith
@@ -1472,8 +1461,8 @@ static inline int startswith(const char *string, const char *pattern)
 static int process_string_cmd(void *user_ctx, const char *data,
                               const GdbCmdParseEntry *cmds, int num_cmds)
 {
-    int i, schema_len, max_num_params = 0;
-    GdbCmdContext gdb_ctx;
+    int i;
+    g_autoptr(GArray) params = g_array_new(false, true, sizeof(GdbCmdVariant));
 
     if (!cmds) {
         return -1;
@@ -1489,24 +1478,13 @@ static int process_string_cmd(void *user_ctx, const char *data,
         }
 
         if (cmd->schema) {
-            schema_len = strlen(cmd->schema);
-            if (schema_len % 2) {
-                return -2;
+            if (cmd_parse_params(&data[strlen(cmd->cmd)],
+                                 cmd->schema, params)) {
+                return -1;
             }
-
-            max_num_params = schema_len / 2;
-        }
-
-        gdb_ctx.params =
-            (GdbCmdVariant *)alloca(sizeof(*gdb_ctx.params) * max_num_params);
-        memset(gdb_ctx.params, 0, sizeof(*gdb_ctx.params) * max_num_params);
-
-        if (cmd_parse_params(&data[strlen(cmd->cmd)], cmd->schema,
-                             gdb_ctx.params, &gdb_ctx.num_params)) {
-            return -1;
         }
 
-        cmd->handler(&gdb_ctx, user_ctx);
+        cmd->handler(params, user_ctx);
         return 0;
     }
 
@@ -1529,18 +1507,18 @@ static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
     }
 }
 
-static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_detach(GArray *params, void *user_ctx)
 {
     GDBProcess *process;
     uint32_t pid = 1;
 
     if (gdbserver_state.multiprocess) {
-        if (!gdb_ctx->num_params) {
+        if (!params->len) {
             put_packet("E22");
             return;
         }
 
-        pid = gdb_ctx->params[0].val_ul;
+        pid = get_param(params, 0)->val_ul;
     }
 
     process = gdb_get_process(pid);
@@ -1563,22 +1541,22 @@ static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet("OK");
 }
 
-static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_thread_alive(GArray *params, void *user_ctx)
 {
     CPUState *cpu;
 
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         put_packet("E22");
         return;
     }
 
-    if (gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
+    if (get_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) {
         put_packet("E22");
         return;
     }
 
-    cpu = gdb_get_cpu(gdb_ctx->params[0].thread_id.pid,
-                      gdb_ctx->params[0].thread_id.tid);
+    cpu = gdb_get_cpu(get_param(params, 0)->thread_id.pid,
+                      get_param(params, 0)->thread_id.tid);
     if (!cpu) {
         put_packet("E22");
         return;
@@ -1587,17 +1565,17 @@ static void handle_thread_alive(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet("OK");
 }
 
-static void handle_continue(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_continue(GArray *params, void *user_ctx)
 {
-    if (gdb_ctx->num_params) {
-        gdb_set_cpu_pc(gdb_ctx->params[0].val_ull);
+    if (params->len) {
+        gdb_set_cpu_pc(get_param(params, 0)->val_ull);
     }
 
     gdbserver_state.signal = 0;
     gdb_continue();
 }
 
-static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_cont_with_sig(GArray *params, void *user_ctx)
 {
     unsigned long signal = 0;
 
@@ -1605,8 +1583,8 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
      * Note: C sig;[addr] is currently unsupported and we simply
      *       omit the addr parameter
      */
-    if (gdb_ctx->num_params) {
-        signal = gdb_ctx->params[0].val_ul;
+    if (params->len) {
+        signal = get_param(params, 0)->val_ul;
     }
 
     gdbserver_state.signal = gdb_signal_to_target(signal);
@@ -1616,27 +1594,27 @@ static void handle_cont_with_sig(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_continue();
 }
 
-static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_set_thread(GArray *params, void *user_ctx)
 {
     CPUState *cpu;
 
-    if (gdb_ctx->num_params != 2) {
+    if (params->len != 2) {
         put_packet("E22");
         return;
     }
 
-    if (gdb_ctx->params[1].thread_id.kind == GDB_READ_THREAD_ERR) {
+    if (get_param(params, 1)->thread_id.kind == GDB_READ_THREAD_ERR) {
         put_packet("E22");
         return;
     }
 
-    if (gdb_ctx->params[1].thread_id.kind != GDB_ONE_THREAD) {
+    if (get_param(params, 1)->thread_id.kind != GDB_ONE_THREAD) {
         put_packet("OK");
         return;
     }
 
-    cpu = gdb_get_cpu(gdb_ctx->params[1].thread_id.pid,
-                      gdb_ctx->params[1].thread_id.tid);
+    cpu = gdb_get_cpu(get_param(params, 1)->thread_id.pid,
+                      get_param(params, 1)->thread_id.tid);
     if (!cpu) {
         put_packet("E22");
         return;
@@ -1646,7 +1624,7 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
      * Note: This command is deprecated and modern gdb's will be using the
      *       vCont command instead.
      */
-    switch (gdb_ctx->params[0].opcode) {
+    switch (get_param(params, 0)->opcode) {
     case 'c':
         gdbserver_state.c_cpu = cpu;
         put_packet("OK");
@@ -1661,18 +1639,18 @@ static void handle_set_thread(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 }
 
-static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_insert_bp(GArray *params, void *user_ctx)
 {
     int res;
 
-    if (gdb_ctx->num_params != 3) {
+    if (params->len != 3) {
         put_packet("E22");
         return;
     }
 
-    res = gdb_breakpoint_insert(gdb_ctx->params[0].val_ul,
-                                gdb_ctx->params[1].val_ull,
-                                gdb_ctx->params[2].val_ull);
+    res = gdb_breakpoint_insert(get_param(params, 0)->val_ul,
+                                get_param(params, 1)->val_ull,
+                                get_param(params, 2)->val_ull);
     if (res >= 0) {
         put_packet("OK");
         return;
@@ -1684,18 +1662,18 @@ static void handle_insert_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet("E22");
 }
 
-static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_remove_bp(GArray *params, void *user_ctx)
 {
     int res;
 
-    if (gdb_ctx->num_params != 3) {
+    if (params->len != 3) {
         put_packet("E22");
         return;
     }
 
-    res = gdb_breakpoint_remove(gdb_ctx->params[0].val_ul,
-                                gdb_ctx->params[1].val_ull,
-                                gdb_ctx->params[2].val_ull);
+    res = gdb_breakpoint_remove(get_param(params, 0)->val_ul,
+                                get_param(params, 1)->val_ull,
+                                get_param(params, 2)->val_ull);
     if (res >= 0) {
         put_packet("OK");
         return;
@@ -1718,7 +1696,7 @@ static void handle_remove_bp(GdbCmdContext *gdb_ctx, void *user_ctx)
  * the remote gdb to fallback to older methods.
  */
 
-static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_set_reg(GArray *params, void *user_ctx)
 {
     int reg_size;
 
@@ -1727,19 +1705,19 @@ static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    if (gdb_ctx->num_params != 2) {
+    if (params->len != 2) {
         put_packet("E22");
         return;
     }
 
-    reg_size = strlen(gdb_ctx->params[1].data) / 2;
-    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[1].data, reg_size);
+    reg_size = strlen(get_param(params, 1)->data) / 2;
+    hextomem(gdbserver_state.mem_buf, get_param(params, 1)->data, reg_size);
     gdb_write_register(gdbserver_state.g_cpu, gdbserver_state.mem_buf->data,
-                       gdb_ctx->params[0].val_ull);
+                       get_param(params, 0)->val_ull);
     put_packet("OK");
 }
 
-static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_get_reg(GArray *params, void *user_ctx)
 {
     int reg_size;
 
@@ -1748,14 +1726,14 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
         return;
     }
 
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         put_packet("E14");
         return;
     }
 
     reg_size = gdb_read_register(gdbserver_state.g_cpu,
                                  gdbserver_state.mem_buf,
-                                 gdb_ctx->params[0].val_ull);
+                                 get_param(params, 0)->val_ull);
     if (!reg_size) {
         put_packet("E14");
         return;
@@ -1767,22 +1745,22 @@ static void handle_get_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_strbuf();
 }
 
-static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_write_mem(GArray *params, void *user_ctx)
 {
-    if (gdb_ctx->num_params != 3) {
+    if (params->len != 3) {
         put_packet("E22");
         return;
     }
 
     /* hextomem() reads 2*len bytes */
-    if (gdb_ctx->params[1].val_ull > strlen(gdb_ctx->params[2].data) / 2) {
+    if (get_param(params, 1)->val_ull > strlen(get_param(params, 2)->data) / 2) {
         put_packet("E22");
         return;
     }
 
-    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[2].data,
-             gdb_ctx->params[1].val_ull);
-    if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,
+    hextomem(gdbserver_state.mem_buf, get_param(params, 2)->data,
+             get_param(params, 1)->val_ull);
+    if (target_memory_rw_debug(gdbserver_state.g_cpu, get_param(params, 0)->val_ull,
                                gdbserver_state.mem_buf->data,
                                gdbserver_state.mem_buf->len, true)) {
         put_packet("E14");
@@ -1792,22 +1770,22 @@ static void handle_write_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet("OK");
 }
 
-static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_read_mem(GArray *params, void *user_ctx)
 {
-    if (gdb_ctx->num_params != 2) {
+    if (params->len != 2) {
         put_packet("E22");
         return;
     }
 
     /* memtohex() doubles the required space */
-    if (gdb_ctx->params[1].val_ull > MAX_PACKET_LENGTH / 2) {
+    if (get_param(params, 1)->val_ull > MAX_PACKET_LENGTH / 2) {
         put_packet("E22");
         return;
     }
 
-    g_byte_array_set_size(gdbserver_state.mem_buf, gdb_ctx->params[1].val_ull);
+    g_byte_array_set_size(gdbserver_state.mem_buf, get_param(params, 1)->val_ull);
 
-    if (target_memory_rw_debug(gdbserver_state.g_cpu, gdb_ctx->params[0].val_ull,
+    if (target_memory_rw_debug(gdbserver_state.g_cpu, get_param(params, 0)->val_ull,
                                gdbserver_state.mem_buf->data,
                                gdbserver_state.mem_buf->len, false)) {
         put_packet("E14");
@@ -1819,19 +1797,19 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_strbuf();
 }
 
-static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_write_all_regs(GArray *params, void *user_ctx)
 {
     target_ulong addr, len;
     uint8_t *registers;
     int reg_size;
 
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         return;
     }
 
     cpu_synchronize_state(gdbserver_state.g_cpu);
-    len = strlen(gdb_ctx->params[0].data) / 2;
-    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len);
+    len = strlen(get_param(params, 0)->data) / 2;
+    hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len);
     registers = gdbserver_state.mem_buf->data;
     for (addr = 0; addr < gdbserver_state.g_cpu->gdb_num_g_regs && len > 0;
          addr++) {
@@ -1842,7 +1820,7 @@ static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet("OK");
 }
 
-static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_read_all_regs(GArray *params, void *user_ctx)
 {
     target_ulong addr, len;
 
@@ -1860,14 +1838,14 @@ static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_strbuf();
 }
 
-static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_file_io(GArray *params, void *user_ctx)
 {
-    if (gdb_ctx->num_params >= 1 && gdbserver_state.current_syscall_cb) {
+    if (params->len >= 1 && gdbserver_state.current_syscall_cb) {
         target_ulong ret, err;
 
-        ret = (target_ulong)gdb_ctx->params[0].val_ull;
-        if (gdb_ctx->num_params >= 2) {
-            err = (target_ulong)gdb_ctx->params[1].val_ull;
+        ret = (target_ulong)get_param(params, 0)->val_ull;
+        if (params->len >= 2) {
+            err = (target_ulong)get_param(params, 1)->val_ull;
         } else {
             err = 0;
         }
@@ -1875,7 +1853,7 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
         gdbserver_state.current_syscall_cb = NULL;
     }
 
-    if (gdb_ctx->num_params >= 3 && gdb_ctx->params[2].opcode == (uint8_t)'C') {
+    if (params->len >= 3 && get_param(params, 2)->opcode == (uint8_t)'C') {
         put_packet("T02");
         return;
     }
@@ -1883,23 +1861,23 @@ static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdb_continue();
 }
 
-static void handle_step(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_step(GArray *params, void *user_ctx)
 {
-    if (gdb_ctx->num_params) {
-        gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull);
+    if (params->len) {
+        gdb_set_cpu_pc((target_ulong)get_param(params, 0)->val_ull);
     }
 
     cpu_single_step(gdbserver_state.c_cpu, get_sstep_flags());
     gdb_continue();
 }
 
-static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_backward(GArray *params, void *user_ctx)
 {
     if (replay_mode != REPLAY_MODE_PLAY) {
         put_packet("E22");
     }
-    if (gdb_ctx->num_params == 1) {
-        switch (gdb_ctx->params[0].opcode) {
+    if (params->len == 1) {
+        switch (get_param(params, 0)->opcode) {
         case 's':
             if (replay_reverse_step()) {
                 gdb_continue();
@@ -1921,20 +1899,20 @@ static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_packet("");
 }
 
-static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_v_cont_query(GArray *params, void *user_ctx)
 {
     put_packet("vCont;c;C;s;S");
 }
 
-static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_v_cont(GArray *params, void *user_ctx)
 {
     int res;
 
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         return;
     }
 
-    res = gdb_handle_vcont(gdb_ctx->params[0].data);
+    res = gdb_handle_vcont(get_param(params, 0)->data);
     if ((res == -EINVAL) || (res == -ERANGE)) {
         put_packet("E22");
     } else if (res) {
@@ -1942,17 +1920,17 @@ static void handle_v_cont(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 }
 
-static void handle_v_attach(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_v_attach(GArray *params, void *user_ctx)
 {
     GDBProcess *process;
     CPUState *cpu;
 
     g_string_assign(gdbserver_state.str_buf, "E22");
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         goto cleanup;
     }
 
-    process = gdb_get_process(gdb_ctx->params[0].val_ul);
+    process = gdb_get_process(get_param(params, 0)->val_ul);
     if (!process) {
         goto cleanup;
     }
@@ -1973,7 +1951,7 @@ cleanup:
     put_strbuf();
 }
 
-static void handle_v_kill(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_v_kill(GArray *params, void *user_ctx)
 {
     /* Kill the target */
     put_packet("OK");
@@ -2008,43 +1986,43 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = {
     },
 };
 
-static void handle_v_commands(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_v_commands(GArray *params, void *user_ctx)
 {
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         return;
     }
 
-    if (process_string_cmd(NULL, gdb_ctx->params[0].data,
+    if (process_string_cmd(NULL, get_param(params, 0)->data,
                            gdb_v_commands_table,
                            ARRAY_SIZE(gdb_v_commands_table))) {
         put_packet("");
     }
 }
 
-static void handle_query_qemu_sstepbits(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_qemu_sstepbits(GArray *params, void *user_ctx)
 {
     g_string_printf(gdbserver_state.str_buf, "ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
                     SSTEP_ENABLE, SSTEP_NOIRQ, SSTEP_NOTIMER);
     put_strbuf();
 }
 
-static void handle_set_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_set_qemu_sstep(GArray *params, void *user_ctx)
 {
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         return;
     }
 
-    sstep_flags = gdb_ctx->params[0].val_ul;
+    sstep_flags = get_param(params, 0)->val_ul;
     put_packet("OK");
 }
 
-static void handle_query_qemu_sstep(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_qemu_sstep(GArray *params, void *user_ctx)
 {
     g_string_printf(gdbserver_state.str_buf, "0x%x", sstep_flags);
     put_strbuf();
 }
 
-static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_curr_tid(GArray *params, void *user_ctx)
 {
     CPUState *cpu;
     GDBProcess *process;
@@ -2061,7 +2039,7 @@ static void handle_query_curr_tid(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_strbuf();
 }
 
-static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_threads(GArray *params, void *user_ctx)
 {
     if (!gdbserver_state.query_cpu) {
         put_packet("l");
@@ -2074,25 +2052,25 @@ static void handle_query_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
     gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
 }
 
-static void handle_query_first_threads(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_first_threads(GArray *params, void *user_ctx)
 {
     gdbserver_state.query_cpu = gdb_first_attached_cpu();
-    handle_query_threads(gdb_ctx, user_ctx);
+    handle_query_threads(params, user_ctx);
 }
 
-static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_thread_extra(GArray *params, void *user_ctx)
 {
     g_autoptr(GString) rs = g_string_new(NULL);
     CPUState *cpu;
 
-    if (!gdb_ctx->num_params ||
-        gdb_ctx->params[0].thread_id.kind == GDB_READ_THREAD_ERR) {
+    if (!params->len ||
+        get_param(params, 0)->thread_id.kind == GDB_READ_THREAD_ERR) {
         put_packet("E22");
         return;
     }
 
-    cpu = gdb_get_cpu(gdb_ctx->params[0].thread_id.pid,
-                      gdb_ctx->params[0].thread_id.tid);
+    cpu = gdb_get_cpu(get_param(params, 0)->thread_id.pid,
+                      get_param(params, 0)->thread_id.tid);
     if (!cpu) {
         return;
     }
@@ -2117,7 +2095,7 @@ static void handle_query_thread_extra(GdbCmdContext *gdb_ctx, void *user_ctx)
 }
 
 #ifdef CONFIG_USER_ONLY
-static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_offsets(GArray *params, void *user_ctx)
 {
     TaskState *ts;
 
@@ -2132,17 +2110,17 @@ static void handle_query_offsets(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_strbuf();
 }
 #else
-static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_rcmd(GArray *params, void *user_ctx)
 {
     const guint8 zero = 0;
     int len;
 
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         put_packet("E22");
         return;
     }
 
-    len = strlen(gdb_ctx->params[0].data);
+    len = strlen(get_param(params, 0)->data);
     if (len % 2) {
         put_packet("E01");
         return;
@@ -2150,7 +2128,7 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
 
     g_assert(gdbserver_state.mem_buf->len == 0);
     len = len / 2;
-    hextomem(gdbserver_state.mem_buf, gdb_ctx->params[0].data, len);
+    hextomem(gdbserver_state.mem_buf, get_param(params, 0)->data, len);
     g_byte_array_append(gdbserver_state.mem_buf, &zero, 1);
     qemu_chr_be_write(gdbserver_state.mon_chr, gdbserver_state.mem_buf->data,
                       gdbserver_state.mem_buf->len);
@@ -2158,7 +2136,7 @@ static void handle_query_rcmd(GdbCmdContext *gdb_ctx, void *user_ctx)
 }
 #endif
 
-static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_supported(GArray *params, void *user_ctx)
 {
     CPUClass *cc;
 
@@ -2179,8 +2157,8 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 #endif
 
-    if (gdb_ctx->num_params &&
-        strstr(gdb_ctx->params[0].data, "multiprocess+")) {
+    if (params->len &&
+        strstr(get_param(params, 0)->data, "multiprocess+")) {
         gdbserver_state.multiprocess = true;
     }
 
@@ -2188,7 +2166,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
     put_strbuf();
 }
 
-static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_xfer_features(GArray *params, void *user_ctx)
 {
     GDBProcess *process;
     CPUClass *cc;
@@ -2196,7 +2174,7 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
     const char *xml;
     const char *p;
 
-    if (gdb_ctx->num_params < 3) {
+    if (params->len < 3) {
         put_packet("E22");
         return;
     }
@@ -2209,15 +2187,15 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
     }
 
     gdb_has_xml = true;
-    p = gdb_ctx->params[0].data;
+    p = get_param(params, 0)->data;
     xml = get_feature_xml(p, &p, process);
     if (!xml) {
         put_packet("E00");
         return;
     }
 
-    addr = gdb_ctx->params[1].val_ul;
-    len = gdb_ctx->params[2].val_ul;
+    addr = get_param(params, 1)->val_ul;
+    len = get_param(params, 2)->val_ul;
     total_len = strlen(xml);
     if (addr > total_len) {
         put_packet("E00");
@@ -2241,18 +2219,18 @@ static void handle_query_xfer_features(GdbCmdContext *gdb_ctx, void *user_ctx)
 }
 
 #if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX_USER)
-static void handle_query_xfer_auxv(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_xfer_auxv(GArray *params, void *user_ctx)
 {
     TaskState *ts;
     unsigned long offset, len, saved_auxv, auxv_len;
 
-    if (gdb_ctx->num_params < 2) {
+    if (params->len < 2) {
         put_packet("E22");
         return;
     }
 
-    offset = gdb_ctx->params[0].val_ul;
-    len = gdb_ctx->params[1].val_ul;
+    offset = get_param(params, 0)->val_ul;
+    len = get_param(params, 1)->val_ul;
     ts = gdbserver_state.c_cpu->opaque;
     saved_auxv = ts->info->saved_auxv;
     auxv_len = ts->info->auxv_len;
@@ -2287,12 +2265,12 @@ static void handle_query_xfer_auxv(GdbCmdContext *gdb_ctx, void *user_ctx)
 }
 #endif
 
-static void handle_query_attached(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_attached(GArray *params, void *user_ctx)
 {
     put_packet(GDB_ATTACHED);
 }
 
-static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_query_qemu_supported(GArray *params, void *user_ctx)
 {
     g_string_printf(gdbserver_state.str_buf, "sstepbits;sstep");
 #ifndef CONFIG_USER_ONLY
@@ -2302,21 +2280,21 @@ static void handle_query_qemu_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
 }
 
 #ifndef CONFIG_USER_ONLY
-static void handle_query_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx,
+static void handle_query_qemu_phy_mem_mode(GArray *params,
                                            void *user_ctx)
 {
     g_string_printf(gdbserver_state.str_buf, "%d", phy_memory_mode);
     put_strbuf();
 }
 
-static void handle_set_qemu_phy_mem_mode(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_set_qemu_phy_mem_mode(GArray *params, void *user_ctx)
 {
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         put_packet("E22");
         return;
     }
 
-    if (!gdb_ctx->params[0].val_ul) {
+    if (!get_param(params, 0)->val_ul) {
         phy_memory_mode = 0;
     } else {
         phy_memory_mode = 1;
@@ -2439,45 +2417,45 @@ static const GdbCmdParseEntry gdb_gen_set_table[] = {
 #endif
 };
 
-static void handle_gen_query(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_gen_query(GArray *params, void *user_ctx)
 {
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         return;
     }
 
-    if (!process_string_cmd(NULL, gdb_ctx->params[0].data,
+    if (!process_string_cmd(NULL, get_param(params, 0)->data,
                             gdb_gen_query_set_common_table,
                             ARRAY_SIZE(gdb_gen_query_set_common_table))) {
         return;
     }
 
-    if (process_string_cmd(NULL, gdb_ctx->params[0].data,
+    if (process_string_cmd(NULL, get_param(params, 0)->data,
                            gdb_gen_query_table,
                            ARRAY_SIZE(gdb_gen_query_table))) {
         put_packet("");
     }
 }
 
-static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_gen_set(GArray *params, void *user_ctx)
 {
-    if (!gdb_ctx->num_params) {
+    if (!params->len) {
         return;
     }
 
-    if (!process_string_cmd(NULL, gdb_ctx->params[0].data,
+    if (!process_string_cmd(NULL, get_param(params, 0)->data,
                             gdb_gen_query_set_common_table,
                             ARRAY_SIZE(gdb_gen_query_set_common_table))) {
         return;
     }
 
-    if (process_string_cmd(NULL, gdb_ctx->params[0].data,
+    if (process_string_cmd(NULL, get_param(params, 0)->data,
                            gdb_gen_set_table,
                            ARRAY_SIZE(gdb_gen_set_table))) {
         put_packet("");
     }
 }
 
-static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
+static void handle_target_halt(GArray *params, void *user_ctx)
 {
     g_string_printf(gdbserver_state.str_buf, "T%02xthread:", GDB_SIGNAL_TRAP);
     gdb_append_thread_id(gdbserver_state.c_cpu, gdbserver_state.str_buf);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-05-06 16:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-05 17:00 [PATCH 0/5] misc: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
2021-05-05 17:00 ` [PATCH 1/5] bsd-user/syscall: Replace alloca() by g_new() Philippe Mathieu-Daudé
2021-05-05 17:00 ` [PATCH 2/5] gdbstub: Constify GdbCmdParseEntry Philippe Mathieu-Daudé
2021-05-06 11:50   ` Alex Bennée
2021-05-05 17:00 ` [PATCH 3/5] gdbstub: Use fixed-size array in GdbCmdParseEntry instead of pointer Philippe Mathieu-Daudé
2021-05-06 12:01   ` Alex Bennée
2021-05-06 12:39     ` Philippe Mathieu-Daudé
2021-05-06 15:15       ` Alex Bennée
2021-05-05 17:00 ` [PATCH 4/5] gdbstub: Replace alloca() by g_new() Philippe Mathieu-Daudé
2021-05-06 16:07   ` [ALT PATCH] gdbstub: Replace GdbCmdContext with plain g_array() Alex Bennée
2021-05-05 17:00 ` [PATCH 5/5] target/ppc/kvm: Replace alloca() by g_malloc() Philippe Mathieu-Daudé
2021-05-06  2:18   ` David Gibson
2021-05-06  8:41   ` Greg Kurz
2021-05-06 13:09     ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).