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

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

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.

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