This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] completion in command definition

Hi Jerome,

I can comment on styling issues (some of them nits), but not so much on the actual problem, since I'm not very familiar with it. But in general it cleans up the code nicely I think.

diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index e5ab839..83bc34a 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -253,4 +253,5 @@ extern const char * const auto_boolean_enums[];

 extern int cli_user_command_p (struct cmd_list_element *);

+extern int find_command_name_length (const char *);

Add new line here?

 #endif /* !defined (CLI_DECODE_H) */
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 6f44d52..338d726 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -904,6 +904,28 @@ read_next_line (void)
   return command_line_input (prompt_ptr, from_tty, "commands");

+/* Return non-zero if CMD's name is NAME.  */
+static int
+command_name_equals (struct cmd_list_element *cmd, char *name)

This can return a bool, and the command can say true instead of non-zero.

name can be "const char *".

+  return cmd

For null pointer checks, use "cmd != NULL" or "cmd != nullptr".

+    && cmd != CMD_LIST_AMBIGUOUS
+    && strcmp (cmd->name, name) == 0;
+/* Given an input line P, skip the command and return a pointer to the
+   first argument.  */
+static char *
+line_first_arg (char *p)

The argument type and return type can be const. It will require you to constify a few other things which the compiler will tell you.

+  char *first_arg = p + find_command_name_length (p);

Add an empty line here...

+  while (first_arg != '\0' && isspace (*first_arg))
+    first_arg++;

... and here.

+  return first_arg;
 /* Process one input line.  If the command is an "end", return such an
    indication to the caller.  If PARSE_COMMANDS is true, strip leading
    whitespace (trailing whitespace is always stripped) in the line,
@@ -919,6 +941,9 @@ process_next_line (char *p, struct command_line
**command, int parse_commands,
   char *p_end;
   char *p_start;
   int not_handled = 0;
+  char *cmd_name = p;

This can be constified.

+  struct cmd_list_element *last_line = 0;

This should be either "= NULL" or "= nullptr", but actually I don't think it needs to be initialized.

+  struct cmd_list_element *cmd;

   /* Not sure what to do here.  */
   if (p == NULL)
@@ -938,9 +963,11 @@ process_next_line (char *p, struct command_line
**command, int parse_commands,
      We also permit whitespace before end and after.  */
   if (p_end - p_start == 3 && startswith (p_start, "end"))
     return end_command;
   if (parse_commands)
+ cmd = lookup_cmd_1 ((const char **) &cmd_name, cmdlist, &last_line, 1);

If cmd_name is constified, the const char cast can be removed.

diff --git a/gdb/testsuite/gdb.base/define.exp
index 59203ec..355a7bc 100644
--- a/gdb/testsuite/gdb.base/define.exp
+++ b/gdb/testsuite/gdb.base/define.exp
@@ -147,6 +147,27 @@ gdb_test_multiple "define ifnospace" "define user
command: ifnospace" \

gdb_test "ifnospace" ".*hi there.*" "test ifnospace is parsed correctly"

+# Verify that the command parser properly handle completion.
+gdb_test_multiple "define breakmain" "define user command: breakmain" \
+  -re "Type commands for definition of \"breakmain\".\r\nEnd with a
line saying just \"end\".\r\n>$" \
+    {
+      gdb_test_multiple "break main\ncommand\necho\nend\nend" "send
body of breakmain"  \
+        {
+         -re "$gdb_prompt $"\
+                 {pass "define user command: breakmain"}
+        }
+    }

I think it's preferable to use the same test name in the gdb_test_multiple and in the pass. Usually, we define a variable with the test name and use it for both. Also, if there are two gdb_test_multiple, I guess we would need two "pass"? Like:

set test "define user command: breakmain"
gdb_test_multiple ... $test {
  -re ... {
    pass $test

    set test "send body of breakmain"
    gdb_test_multiple ... $test {
      -re ... {
        pass $test

I'm not sure about that...



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]