Re: [PATCH] PR-10034 Bad space handling in `set remote exec-file' command.

[Tom Tromey <tromey at redhat dot 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