This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] PR-10034 Bad space handling in `set remote exec-file' command.
- From: Tom Tromey <tromey at redhat dot com>
- To: Abhijit Halder <abhijit dot k dot halder at gmail dot com>
- Cc: "gdb-patches\ at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Tue, 04 Oct 2011 09:50:46 -0600
- Subject: Re: [PATCH] PR-10034 Bad space handling in `set remote exec-file' command.
- References: <CAOhZP9wi+uB80QyjN4Fv9W2ksX19ScBf1Z+Y15LQq0-oq8A0qA@mail.gmail.com>
>>>>> "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.
This patch doesn't have a test case, but should.
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?
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.
What happens if the resulting string is empty?
At least for var_filename this should give an error.
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.
Tom