This is the mail archive of the gdb-patches@sourceware.org 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] |
On Fri, Oct 14, 2011 at 3:23 PM, Abhijit Halder <abhijit.k.halder@gmail.com> wrote: > On Wed, Oct 5, 2011 at 9:48 AM, Abhijit Halder > <abhijit.k.halder@gmail.com> wrote: >> On Tue, Oct 4, 2011 at 9:20 PM, Tom Tromey <tromey@redhat.com> wrote: >>>>>>>> "Abhijit" == Abhijit Halder <abhijit.k.halder@gmail.com> writes: >>> >>> Abhijit> In the `set remote exec-file' command if we provide space at the end >>> Abhijit> of the file-name, the space is not being chopped off and being >>> Abhijit> considered as part of file-name. This behavior is inconsistent across >>> Abhijit> similar set commands like `set logging file' etc. My patch will fix >>> Abhijit> that problem. Please review this patch and put your comments. >>> >>> I can't tell if you re-posted the same patch or if it has changes. >>> Please: >>> >>> * If you are sending a ping, just send a ping, as a followup to the >>> ?patch being pinged, so that it threads properly in a threading mail >>> ?reader. >>> >>> * If you are sending a new patch, again send it as a followup, and also >>> ?indicate what changed and why. >>> >> Okay, I will follow this in future. >>> >>> This patch doesn't have a test case, but should. >>> >> Okay, I will write a test-case. >>> Abhijit> ? ? ? ?case var_string_noescape: >>> Abhijit> - ? ? ? ?if (arg == NULL) >>> Abhijit> - ? ? ? ? ?arg = ""; >>> Abhijit> - ? ? ? ?if (*(char **) c->var != NULL) >>> Abhijit> - ? ? ? ? ?xfree (*(char **) c->var); >>> Abhijit> - ? ? ? ?*(char **) c->var = xstrdup (arg); >>> Abhijit> - ? ? ? ?break; >>> >>> Why are you changing this case? >>> >> The idea was that the same code block is common between two adjacent >> cases and hence can be combined together. >>> Abhijit> ? ? ? ?case var_optional_filename: >>> Abhijit> ? ? ? ? ?if (arg == NULL) >>> Abhijit> ? ? ? ? ? ?arg = ""; >>> Abhijit> + ? ? ? ?else >>> Abhijit> + ? ? ? ? ?{ >>> Abhijit> + ? ? ? ? ? ?/* Clear trailing whitespace. ?*/ >>> Abhijit> + ? ? ? ? ? ?char *ptr = arg + strlen (arg) - 1; >>> Abhijit> + >>> Abhijit> + ? ? ? ? ? ?while (ptr >= arg && (*ptr == ' ' || *ptr == '\t')) >>> Abhijit> + ? ? ? ? ? ? ?ptr--; >>> Abhijit> + ? ? ? ? ? ?*(ptr + 1) = '\0'; >>> >>> Why not use remove_trailing_whitespace? ?You mentioned it in an earlier >>> note, it seems best to just use it from the start. > Since this same function has similar code block in several places used > for the purpose of removing trailing whiltespaces, I think now we > should go with the current approach of using this code block instead > of using remove_trailing_whitespace function. This may cause > confusion. Please comment on this. >>> >> Okay, I will do this change. I was just waiting for people's comment on this. >>> >>> What happens if the resulting string is empty? >>> At least for var_filename this should give an error. >>> > I did not make any change for var_filename in my patch. As in existing > code in the patch also the empty string will throw error. >> Okay. >>> Abhijit> @@ -419,7 +423,7 @@ cmd_show_list (struct cmd_list_element * >>> Abhijit> ? ?for (; list != NULL; list = list->next) >>> Abhijit> ? ? ?{ >>> Abhijit> ? ? ? ?/* If we find a prefix, run its list, prefixing our output by its >>> Abhijit> - ? ? ? ? prefix (with "show " skipped). ?*/ >>> Abhijit> + ? ? ? prefix (with "show " skipped). ?*/ >>> Abhijit> ? ? ? ?if (list->prefixlist && !list->abbrev_flag) >>> Abhijit> ? ? ? ?{ >>> Abhijit> ? ? ? ? ?struct cleanup *optionlist_chain >>> >>> This hunk looks like a gratuitous change. >>> >> This is a 8 space to tab conversion that we followed in rest of the >> code. If suggested I may revert back this change. >>> >>> Tom >>> >> >> Regards, >> Abhijit Halder >> > Adding testcases. Regards, Abhijit Halder
Attachment:
ChangeLog-testsuite.txt
Description: Text document
Index: gdb.base/setshow.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v retrieving revision 1.21 diff -a -p -u -r1.21 setshow.exp --- gdb.base/setshow.exp 20 Apr 2011 14:56:49 -0000 1.21 +++ gdb.base/setshow.exp 14 Oct 2011 11:33:49 -0000 @@ -260,3 +260,7 @@ gdb_test "show verbose" "Verbose printin gdb_test_no_output "set verbose off" "set verbose off" #test show verbose off gdb_test "show verbose" "Verbosity is off..*" "show verbose (off)" +#test set remote exec-file +gdb_test_no_output "set remote exec-file xxx " +#test show remote exec-file +gdb_test "show remote exec-file" "The remote pathname for \"run\" is \"xxx\"."
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |