* [PATCH] Provide useful completer for "info registers" @ 2014-11-25 17:28 Andreas Arnez 2014-11-26 20:54 ` Sergio Durigan Junior 2014-12-04 17:38 ` Pedro Alves 0 siblings, 2 replies; 10+ messages in thread From: Andreas Arnez @ 2014-11-25 17:28 UTC (permalink / raw) To: gdb-patches Provide a new completion function for the argument of "info registers", "info all-registers", and the "lr" command in dbx mode. Without this patch the default symbol completer is used, which is more confusing than helpful. gdb/ChangeLog: * completer.c: Include "target.h" and "reggroups.h". (reg_or_group_completer): New. * completer.h (reg_or_group_completer): Declare. * infcmd.c (_initialize_infcmd): Set reg_or_group_completer for the "info registers" and "info all-registers" commands and the dbx-mode "lr" command. --- gdb/completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ gdb/completer.h | 3 +++ gdb/infcmd.c | 12 +++++++++--- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/gdb/completer.c b/gdb/completer.c index a0f3fa3..42188c0 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -23,6 +23,8 @@ #include "filenames.h" /* For DOSish file names. */ #include "language.h" #include "gdb_signals.h" +#include "target.h" +#include "reggroups.h" #include "cli/cli-decode.h" @@ -836,6 +838,55 @@ signal_completer (struct cmd_list_element *ignore, return return_val; } +/* Complete on a register or reggroup. */ + +VEC (char_ptr) * +reg_or_group_completer (struct cmd_list_element *ignore, + const char *text, const char *word) +{ + VEC (char_ptr) *result = NULL; + size_t len = strlen (text); + struct frame_info *frame; + struct gdbarch *gdbarch; + + if (!target_has_registers) + return result; + + frame = get_selected_frame (NULL); + gdbarch = get_frame_arch (frame); + + { + int i; + int n_regs = (gdbarch_num_regs (gdbarch) + + gdbarch_num_pseudo_regs (gdbarch)); + + for (i = 0; i < n_regs; i++) + { + const char *reg_name = gdbarch_register_name (gdbarch, i); + + if (reg_name != NULL && strncmp (text, reg_name, len) == 0) + VEC_safe_push (char_ptr, result, xstrdup (reg_name)); + } + } + + { + struct reggroup *group; + + for (group = reggroup_next (gdbarch, NULL); + group != NULL; + group = reggroup_next (gdbarch, group)) + { + const char *group_name = reggroup_name (group); + + if (strncmp (text, group_name, len) == 0) + VEC_safe_push (char_ptr, result, xstrdup (group_name)); + } + } + + return result; +} + + /* Get the list of chars that are considered as word breaks for the current command. */ diff --git a/gdb/completer.h b/gdb/completer.h index bc7ed96..5e91030 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -45,6 +45,9 @@ extern VEC (char_ptr) *command_completer (struct cmd_list_element *, extern VEC (char_ptr) *signal_completer (struct cmd_list_element *, const char *, const char *); +extern VEC (char_ptr) *reg_or_group_completer (struct cmd_list_element *, + const char *, const char *); + extern char *get_gdb_completer_quote_characters (void); extern char *gdb_completion_word_break_characters (void); diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 4415b31..de0d24d 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -3235,18 +3235,24 @@ If non-stop mode is enabled, interrupt only the current thread,\n\ otherwise all the threads in the program are stopped. To \n\ interrupt all running threads in non-stop mode, use the -a option.")); - add_info ("registers", nofp_registers_info, _("\ + c = add_info ("registers", nofp_registers_info, _("\ List of integer registers and their contents, for selected stack frame.\n\ Register name as argument means describe only that register.")); add_info_alias ("r", "registers", 1); + set_cmd_completer (c, reg_or_group_completer); if (xdb_commands) - add_com ("lr", class_info, nofp_registers_info, _("\ + { + c = add_com ("lr", class_info, nofp_registers_info, _("\ List of integer registers and their contents, for selected stack frame.\n\ Register name as argument means describe only that register.")); - add_info ("all-registers", all_registers_info, _("\ + set_cmd_completer (c, reg_or_group_completer); + } + + c = add_info ("all-registers", all_registers_info, _("\ List of all registers and their contents, for selected stack frame.\n\ Register name as argument means describe only that register.")); + set_cmd_completer (c, reg_or_group_completer); add_info ("program", program_info, _("Execution status of the program.")); -- 1.7.9.5 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-11-25 17:28 [PATCH] Provide useful completer for "info registers" Andreas Arnez @ 2014-11-26 20:54 ` Sergio Durigan Junior 2014-11-26 21:52 ` Sergio Durigan Junior ` (2 more replies) 2014-12-04 17:38 ` Pedro Alves 1 sibling, 3 replies; 10+ messages in thread From: Sergio Durigan Junior @ 2014-11-26 20:54 UTC (permalink / raw) To: Andreas Arnez; +Cc: gdb-patches On Tuesday, November 25 2014, Andreas Arnez wrote: > Provide a new completion function for the argument of "info > registers", "info all-registers", and the "lr" command in dbx mode. > Without this patch the default symbol completer is used, which is more > confusing than helpful. Thank you for the patch, Andreas! > gdb/ChangeLog: > > * completer.c: Include "target.h" and "reggroups.h". > (reg_or_group_completer): New. > * completer.h (reg_or_group_completer): Declare. > * infcmd.c (_initialize_infcmd): Set reg_or_group_completer for > the "info registers" and "info all-registers" commands and the > dbx-mode "lr" command. > --- > gdb/completer.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ > gdb/completer.h | 3 +++ > gdb/infcmd.c | 12 +++++++++--- > 3 files changed, 63 insertions(+), 3 deletions(-) > > diff --git a/gdb/completer.c b/gdb/completer.c > index a0f3fa3..42188c0 100644 > --- a/gdb/completer.c > +++ b/gdb/completer.c > @@ -23,6 +23,8 @@ > #include "filenames.h" /* For DOSish file names. */ > #include "language.h" > #include "gdb_signals.h" > +#include "target.h" > +#include "reggroups.h" > > #include "cli/cli-decode.h" > > @@ -836,6 +838,55 @@ signal_completer (struct cmd_list_element *ignore, > return return_val; > } > > +/* Complete on a register or reggroup. */ > + > +VEC (char_ptr) * > +reg_or_group_completer (struct cmd_list_element *ignore, > + const char *text, const char *word) > +{ > + VEC (char_ptr) *result = NULL; > + size_t len = strlen (text); Hm, this should be "strlen (word)". The "text" will hold the entire line that is being completed, and "word" will hold just the last word, according to the breaking characters being used for this specific completer. For example, consider: (gdb) info registers rsp es In this case, "text" will be "rsp es", and "word" will be "es". Most of the time, you will only be interested in using "word" for the completion. Therefore, the "len" variable should hold "strlen (word)". Also, later in the code you are comparing each register name against "text", but you should be comparing against "word", for the reason explained above. Yeah, it can be confusing :-/. > + struct frame_info *frame; > + struct gdbarch *gdbarch; > + > + if (!target_has_registers) > + return result; > + > + frame = get_selected_frame (NULL); > + gdbarch = get_frame_arch (frame); > + > + { > + int i; > + int n_regs = (gdbarch_num_regs (gdbarch) > + + gdbarch_num_pseudo_regs (gdbarch)); > + > + for (i = 0; i < n_regs; i++) > + { > + const char *reg_name = gdbarch_register_name (gdbarch, i); > + > + if (reg_name != NULL && strncmp (text, reg_name, len) == 0) As said above, here you should do: strncmp (word, reg_name, len) == 0 If you compare against "text", only the first completion will work, i.e., you won't be able to complete more than one register name in the command line. > + VEC_safe_push (char_ptr, result, xstrdup (reg_name)); > + } > + } > + > + { > + struct reggroup *group; > + > + for (group = reggroup_next (gdbarch, NULL); > + group != NULL; > + group = reggroup_next (gdbarch, group)) > + { > + const char *group_name = reggroup_name (group); > + > + if (strncmp (text, group_name, len) == 0) Likewise. > + VEC_safe_push (char_ptr, result, xstrdup (group_name)); > + } > + } > + > + return result; > +} While I understand and like this approach, we have a function that does the "strncmp" dance for you. All you need to do is provide a list of possible candidates (char **), and the word being completed. I gave it a try and hacked your patch to do that. The resulting patch is attached, feel free to use it if you like the approach. > + > + > /* Get the list of chars that are considered as word breaks > for the current command. */ > > diff --git a/gdb/completer.h b/gdb/completer.h > index bc7ed96..5e91030 100644 > --- a/gdb/completer.h > +++ b/gdb/completer.h > @@ -45,6 +45,9 @@ extern VEC (char_ptr) *command_completer (struct cmd_list_element *, > extern VEC (char_ptr) *signal_completer (struct cmd_list_element *, > const char *, const char *); > > +extern VEC (char_ptr) *reg_or_group_completer (struct cmd_list_element *, > + const char *, const char *); > + > extern char *get_gdb_completer_quote_characters (void); > > extern char *gdb_completion_word_break_characters (void); > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 4415b31..de0d24d 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -3235,18 +3235,24 @@ If non-stop mode is enabled, interrupt only the current thread,\n\ > otherwise all the threads in the program are stopped. To \n\ > interrupt all running threads in non-stop mode, use the -a option.")); > > - add_info ("registers", nofp_registers_info, _("\ > + c = add_info ("registers", nofp_registers_info, _("\ > List of integer registers and their contents, for selected stack frame.\n\ > Register name as argument means describe only that register.")); > add_info_alias ("r", "registers", 1); > + set_cmd_completer (c, reg_or_group_completer); > > if (xdb_commands) > - add_com ("lr", class_info, nofp_registers_info, _("\ > + { > + c = add_com ("lr", class_info, nofp_registers_info, _("\ > List of integer registers and their contents, for selected stack frame.\n\ > Register name as argument means describe only that register.")); > - add_info ("all-registers", all_registers_info, _("\ > + set_cmd_completer (c, reg_or_group_completer); > + } > + > + c = add_info ("all-registers", all_registers_info, _("\ > List of all registers and their contents, for selected stack frame.\n\ > Register name as argument means describe only that register.")); > + set_cmd_completer (c, reg_or_group_completer); > > add_info ("program", program_info, > _("Execution status of the program.")); > -- > 1.7.9.5 I'd say this patch also needs a testcase :-). I know that this is architecture specific, so I'd personally be happy with something very simple, maybe testing only one or two architectures would be enough. Other than that, it is fine by me (not an approval). Thanks for doing that. -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ Index: binutils-gdb-review-andreas-completer/gdb/completer.c =================================================================== --- binutils-gdb-review-andreas-completer.orig/gdb/completer.c +++ binutils-gdb-review-andreas-completer/gdb/completer.c @@ -844,45 +844,43 @@ VEC (char_ptr) * reg_or_group_completer (struct cmd_list_element *ignore, const char *text, const char *word) { - VEC (char_ptr) *result = NULL; - size_t len = strlen (text); + VEC (char_ptr) *result; struct frame_info *frame; struct gdbarch *gdbarch; + const char **regnames; + int count = 0, n_regs = 0, n_reggroups = 0; + int i; + struct reggroup *group; if (!target_has_registers) - return result; + return NULL; frame = get_selected_frame (NULL); gdbarch = get_frame_arch (frame); - { - int i; - int n_regs = (gdbarch_num_regs (gdbarch) - + gdbarch_num_pseudo_regs (gdbarch)); - - for (i = 0; i < n_regs; i++) - { - const char *reg_name = gdbarch_register_name (gdbarch, i); - - if (reg_name != NULL && strncmp (text, reg_name, len) == 0) - VEC_safe_push (char_ptr, result, xstrdup (reg_name)); - } - } - - { - struct reggroup *group; - - for (group = reggroup_next (gdbarch, NULL); - group != NULL; - group = reggroup_next (gdbarch, group)) - { - const char *group_name = reggroup_name (group); - - if (strncmp (text, group_name, len) == 0) - VEC_safe_push (char_ptr, result, xstrdup (group_name)); - } - } + n_regs = gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch); + for (group = reggroup_next (gdbarch, NULL); + group != NULL; + group = reggroup_next (gdbarch, group), ++n_reggroups); + + regnames = xmalloc ((n_regs + n_reggroups + 1) * sizeof (char *)); + for (i = 0; i < n_regs; i++) + { + const char *reg_name = gdbarch_register_name (gdbarch, i); + + if (reg_name != NULL) + regnames[count++] = reg_name; + } + + for (group = reggroup_next (gdbarch, NULL); + group != NULL; + group = reggroup_next (gdbarch, group)) + regnames[count++] = reggroup_name (group); + regnames[count] = NULL; + + result = complete_on_enum (regnames, text, word); + xfree (regnames); return result; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-11-26 20:54 ` Sergio Durigan Junior @ 2014-11-26 21:52 ` Sergio Durigan Junior 2014-11-28 18:14 ` Andreas Arnez 2014-12-04 17:34 ` Pedro Alves 2 siblings, 0 replies; 10+ messages in thread From: Sergio Durigan Junior @ 2014-11-26 21:52 UTC (permalink / raw) To: Andreas Arnez; +Cc: gdb-patches On Wednesday, November 26 2014, I wrote: [...] > + for (group = reggroup_next (gdbarch, NULL); > + group != NULL; > + group = reggroup_next (gdbarch, group)) > + regnames[count++] = reggroup_name (group); > > + regnames[count] = NULL; > + > + result = complete_on_enum (regnames, text, word); Ops, sorry, this should be: result = complete_on_enum (regnames, word, word); complete_on_enum has such a weird interface that it makes me want to cry... Cheers, -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-11-26 20:54 ` Sergio Durigan Junior 2014-11-26 21:52 ` Sergio Durigan Junior @ 2014-11-28 18:14 ` Andreas Arnez 2014-11-28 20:39 ` Sergio Durigan Junior 2014-12-04 17:34 ` Pedro Alves 2 siblings, 1 reply; 10+ messages in thread From: Andreas Arnez @ 2014-11-28 18:14 UTC (permalink / raw) To: Sergio Durigan Junior; +Cc: gdb-patches On Wed, Nov 26 2014, Sergio Durigan Junior wrote: > On Tuesday, November 25 2014, Andreas Arnez wrote: > >> [...] >> @@ -836,6 +838,55 @@ signal_completer (struct cmd_list_element *ignore, >> return return_val; >> } >> >> +/* Complete on a register or reggroup. */ >> + >> +VEC (char_ptr) * >> +reg_or_group_completer (struct cmd_list_element *ignore, >> + const char *text, const char *word) >> +{ >> + VEC (char_ptr) *result = NULL; >> + size_t len = strlen (text); > > Hm, this should be "strlen (word)". > > The "text" will hold the entire line that is being completed, and "word" > will hold just the last word, according to the breaking characters being > used for this specific completer. For example, consider: > > (gdb) info registers rsp es > > In this case, "text" will be "rsp es", and "word" will be "es". Most of > the time, you will only be interested in using "word" for the > completion. > > Therefore, the "len" variable should hold "strlen (word)". Also, later > in the code you are comparing each register name against "text", but you > should be comparing against "word", for the reason explained above. > > Yeah, it can be confusing :-/. First I actually had used 'word' here, but then I noticed that the completer's notion of words doesn't match how the command parses its arguments. If using 'word', the completer behaves like this: (gdb) complete info registers hello,g info registers hello,general Which I consider a bit strange. However, I realize this may not be a real problem for users, and being able to expand multiple arguments probably beats this flaw, so I'll use 'word', as suggested. > [...] > > While I understand and like this approach, we have a function that does > the "strncmp" dance for you. All you need to do is provide a list of > possible candidates (char **), and the word being completed. I gave it > a try and hacked your patch to do that. The resulting patch is > attached, feel free to use it if you like the approach. Thanks for the patch! Indeed I didn't know about complete_on_enum() before. But after weighing pros and cons, I still prefer the "strncmp dance": It's not longer and needs somewhat less logic, e.g. only two instead of three loops and no temporary xmalloc'd buffer. Also, I think the code is easier to maintain if signal_completer and reg_or_group_completer use the same approach. But since it's a short function, I will dissolve the sub-blocks and move the variable declarations to the top instead, like your patch does. > I'd say this patch also needs a testcase :-). I know that this is > architecture specific, so I'd personally be happy with something very > simple, maybe testing only one or two architectures would be enough. Yes, a test case would probably be adequate. I'll try it in an architecture-independent way and include it in the next version. > Other than that, it is fine by me (not an approval). Thanks for doing > that. Thanks for looking at this, and for your feedback. Much appreciated. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-11-28 18:14 ` Andreas Arnez @ 2014-11-28 20:39 ` Sergio Durigan Junior 0 siblings, 0 replies; 10+ messages in thread From: Sergio Durigan Junior @ 2014-11-28 20:39 UTC (permalink / raw) To: Andreas Arnez; +Cc: gdb-patches On Friday, November 28 2014, Andreas Arnez wrote: >> Hm, this should be "strlen (word)". >> >> The "text" will hold the entire line that is being completed, and "word" >> will hold just the last word, according to the breaking characters being >> used for this specific completer. For example, consider: >> >> (gdb) info registers rsp es >> >> In this case, "text" will be "rsp es", and "word" will be "es". Most of >> the time, you will only be interested in using "word" for the >> completion. >> >> Therefore, the "len" variable should hold "strlen (word)". Also, later >> in the code you are comparing each register name against "text", but you >> should be comparing against "word", for the reason explained above. >> >> Yeah, it can be confusing :-/. > > First I actually had used 'word' here, but then I noticed that the > completer's notion of words doesn't match how the command parses its > arguments. If using 'word', the completer behaves like this: > > (gdb) complete info registers hello,g > info registers hello,general > > Which I consider a bit strange. However, I realize this may not be a > real problem for users, and being able to expand multiple arguments > probably beats this flaw, so I'll use 'word', as suggested. Yeah. This is a problem with our completer scheme; you can see this behavior happening also for other commands: (gdb) complete break hello,ma break hello,mabort break hello,madvise ... And I agree that this may not be a real problem for users (since this problem exists for a long time apparently). >> [...] >> >> While I understand and like this approach, we have a function that does >> the "strncmp" dance for you. All you need to do is provide a list of >> possible candidates (char **), and the word being completed. I gave it >> a try and hacked your patch to do that. The resulting patch is >> attached, feel free to use it if you like the approach. > > Thanks for the patch! Indeed I didn't know about complete_on_enum() > before. But after weighing pros and cons, I still prefer the "strncmp > dance": It's not longer and needs somewhat less logic, e.g. only two > instead of three loops and no temporary xmalloc'd buffer. Also, I think > the code is easier to maintain if signal_completer and > reg_or_group_completer use the same approach. Agreed :-). Although complete_on_enum exists, its interface is in dire need of a revamp; besides, using it on your function does not make the code simpler or clearer as you pointed. > But since it's a short function, I will dissolve the sub-blocks and move > the variable declarations to the top instead, like your patch does. Thanks. >> I'd say this patch also needs a testcase :-). I know that this is >> architecture specific, so I'd personally be happy with something very >> simple, maybe testing only one or two architectures would be enough. > > Yes, a test case would probably be adequate. I'll try it in an > architecture-independent way and include it in the next version. > >> Other than that, it is fine by me (not an approval). Thanks for doing >> that. > > Thanks for looking at this, and for your feedback. Much appreciated. No, thank you! :-) -- Sergio GPG key ID: 0x65FC5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-11-26 20:54 ` Sergio Durigan Junior 2014-11-26 21:52 ` Sergio Durigan Junior 2014-11-28 18:14 ` Andreas Arnez @ 2014-12-04 17:34 ` Pedro Alves 2014-12-10 17:36 ` Andreas Arnez 2 siblings, 1 reply; 10+ messages in thread From: Pedro Alves @ 2014-12-04 17:34 UTC (permalink / raw) To: Sergio Durigan Junior, Andreas Arnez; +Cc: gdb-patches Thanks Andreas, I think a register completer is a great idea. On 11/26/2014 08:54 PM, Sergio Durigan Junior wrote: > I'd say this patch also needs a testcase :-). I know that this is > architecture specific, so I'd personally be happy with something very > simple, maybe testing only one or two architectures would be enough. I think $pc, $sp, $fp (the user regs) should work everywhere. See user-regs.c and std-regs.c. Actually, looks like the patch misses considering those for completion? See infcmd.c:registers_info: /* A register name? */ { int regnum = user_reg_map_name_to_regnum (gdbarch, start, end - start); if (regnum >= 0) { /* User registers lie completely outside of the range of normal registers. Catch them early so that the target never sees them. */ if (regnum >= gdbarch_num_regs (gdbarch) + gdbarch_num_pseudo_regs (gdbarch)) { Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-12-04 17:34 ` Pedro Alves @ 2014-12-10 17:36 ` Andreas Arnez 2014-12-10 18:21 ` Pedro Alves 0 siblings, 1 reply; 10+ messages in thread From: Andreas Arnez @ 2014-12-10 17:36 UTC (permalink / raw) To: Pedro Alves; +Cc: Sergio Durigan Junior, gdb-patches On Thu, Dec 04 2014, Pedro Alves wrote: > Thanks Andreas, > > I think a register completer is a great idea. Thanks, I thought so, too ;-) > > On 11/26/2014 08:54 PM, Sergio Durigan Junior wrote: >> I'd say this patch also needs a testcase :-). I know that this is >> architecture specific, so I'd personally be happy with something very >> simple, maybe testing only one or two architectures would be enough. > > I think $pc, $sp, $fp (the user regs) should work everywhere. > > See user-regs.c and std-regs.c. > > Actually, looks like the patch misses considering those for completion? > > See infcmd.c:registers_info: > > /* A register name? */ > { > int regnum = user_reg_map_name_to_regnum (gdbarch, start, end - start); > > if (regnum >= 0) > { > /* User registers lie completely outside of the range of > normal registers. Catch them early so that the target > never sees them. */ > if (regnum >= gdbarch_num_regs (gdbarch) > + gdbarch_num_pseudo_regs (gdbarch)) > { Yes, the patch misses them... Note that this is consistent with the list of registers shown by "mt print registers" or "info registers all". But you're right, the "info registers" command accepts user registers, so the completer should offer them as well. Added this in v3. It seems that user registers can not be listed by any means in GDB. Most architectures only have the built-in set of user registers ($pc, $sp, $fp, and $ps), but some define additional ones. This creates a difficulty with the test case, which tries to determine the full list of registers and reggroups the completer is supposed to yield. Thus I just added such a command in v3 as well. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-12-10 17:36 ` Andreas Arnez @ 2014-12-10 18:21 ` Pedro Alves 0 siblings, 0 replies; 10+ messages in thread From: Pedro Alves @ 2014-12-10 18:21 UTC (permalink / raw) To: Andreas Arnez; +Cc: Sergio Durigan Junior, gdb-patches On 12/10/2014 05:35 PM, Andreas Arnez wrote: > It seems that user registers can not be listed by any means in GDB. > Most architectures only have the built-in set of user registers ($pc, > $sp, $fp, and $ps), but some define additional ones. This creates a > difficulty with the test case, which tries to determine the full list of > registers and reggroups the completer is supposed to yield. Thus I just > added such a command in v3 as well. Sounds excellent, thanks. Pedro Alves ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-11-25 17:28 [PATCH] Provide useful completer for "info registers" Andreas Arnez 2014-11-26 20:54 ` Sergio Durigan Junior @ 2014-12-04 17:38 ` Pedro Alves 2014-12-10 17:48 ` Andreas Arnez 1 sibling, 1 reply; 10+ messages in thread From: Pedro Alves @ 2014-12-04 17:38 UTC (permalink / raw) To: Andreas Arnez, gdb-patches Also, ... On 11/25/2014 05:28 PM, Andreas Arnez wrote: > + for (i = 0; i < n_regs; i++) > + { > + const char *reg_name = gdbarch_register_name (gdbarch, i); > + > + if (reg_name != NULL && strncmp (text, reg_name, len) == 0) > + VEC_safe_push (char_ptr, result, xstrdup (reg_name)); Not sure under which conditions "len" could be zero here. If it may, note that some registers have empty names. See default_print_registers_info: /* If the register name is empty, it is undefined for this processor, so don't display anything. */ if (gdbarch_register_name (gdbarch, i) == NULL || *(gdbarch_register_name (gdbarch, i)) == '\0') continue; > + } > + } Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Provide useful completer for "info registers" 2014-12-04 17:38 ` Pedro Alves @ 2014-12-10 17:48 ` Andreas Arnez 0 siblings, 0 replies; 10+ messages in thread From: Andreas Arnez @ 2014-12-10 17:48 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On Thu, Dec 04 2014, Pedro Alves wrote: > Also, ... > > On 11/25/2014 05:28 PM, Andreas Arnez wrote: >> + for (i = 0; i < n_regs; i++) >> + { >> + const char *reg_name = gdbarch_register_name (gdbarch, i); >> + >> + if (reg_name != NULL && strncmp (text, reg_name, len) == 0) >> + VEC_safe_push (char_ptr, result, xstrdup (reg_name)); > > Not sure under which conditions "len" could be zero here. If it may, > note that some registers have empty names. See default_print_registers_info: > > /* If the register name is empty, it is undefined for this > processor, so don't display anything. */ > if (gdbarch_register_name (gdbarch, i) == NULL > || *(gdbarch_register_name (gdbarch, i)) == '\0') > continue; > >> + } >> + } Good point. This is fixed in the new version; registers with empty names are skipped now. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-12-10 18:21 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-11-25 17:28 [PATCH] Provide useful completer for "info registers" Andreas Arnez 2014-11-26 20:54 ` Sergio Durigan Junior 2014-11-26 21:52 ` Sergio Durigan Junior 2014-11-28 18:14 ` Andreas Arnez 2014-11-28 20:39 ` Sergio Durigan Junior 2014-12-04 17:34 ` Pedro Alves 2014-12-10 17:36 ` Andreas Arnez 2014-12-10 18:21 ` Pedro Alves 2014-12-04 17:38 ` Pedro Alves 2014-12-10 17:48 ` Andreas Arnez
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 read-only IMAP folder(s) and NNTP newsgroup(s).