| Message ID | 20210505170055.1415360-4-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | misc: Replace alloca() by g_malloc() | expand |
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;
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.
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;
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(-)