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

[Abhijit Halder <abhijit.k.halder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]

On Mon, Sep 26, 2011 at 10:52 AM, Abhijit Halder
<abhijit.k.halder@gmail.com> wrote:
> On Sun, Sep 25, 2011 at 2:21 PM, Abhijit Halder
> <abhijit.k.halder@gmail.com> wrote:
>> Hi,
>>
>> In the set remote exec-file command if we provide space at the end of
>> the file-name the space is not being cleared. This behaviour is
>> inconsistent across similar set commands like set logging file etc. My
>> patch will fix that behaviour. Please review this patch.
>>
>> Further, I have found that there is a function in cli/cli-utils.c
>> called remove_trailing_whitespace that never used. In many times we
>> have removed trailing spaces and for that inline code is written. In
>> my next patch I am planning to modify the remove_trailing_whitespace
>> function and use it whenever possible in that. Since that patch will
>> be relevant to current fix I am proposing, I have mentioned here that
>> point.
>>
>> Thanks,
>> Abhijit Halder
>>
>
> Oops! A mistake. Correcting the same.
>


Index: gdb/cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.46
diff -a -p -u -r1.46 cli-setshow.c
--- gdb/cli/cli-setshow.c	4 Aug 2011 19:10:13 -0000	1.46
+++ gdb/cli/cli-setshow.c	25 Sep 2011 08:28:52 -0000
@@ -181,6 +181,14 @@ do_setshow_command (char *arg, int from_
 	    arg = "";
 	  if (*(char **) c->var != NULL)
 	    xfree (*(char **) c->var);
+	  {
+	    /* Clear trailing whitespace of string.  */
+	    char *ptr = arg + strlen (arg) - 1;
+
+	    while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
+	      ptr--;
+	    *(ptr + 1) = '\0';

>>>>>>>>>>>>Here 'ptr' may point to a read-only memory when 'arg' is passed as NULL pointer.

+	  }
 	  *(char **) c->var = xstrdup (arg);
 	  break;
 	case var_optional_filename:

This case contains same code block as the earlier case. We can fall
through here from earlier case.

@@ -188,6 +196,14 @@ do_setshow_command (char *arg, int from_
 	    arg = "";
 	  if (*(char **) c->var != NULL)
 	    xfree (*(char **) c->var);
+	  {
+	    /* Clear trailing whitespace of filename.  */
+	    char *ptr = arg + strlen (arg) - 1;
+
+	    while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
+	      ptr--;
+	    *(ptr + 1) = '\0';
+	  }
 	  *(char **) c->var = xstrdup (arg);
 	  break;
 	case var_filename:

I have made the corrections. Please ignore the earlier patch sent in hurry.


Thanks,
Abhijit Halder

[-- Attachment #2: ChangeLog.txt --]
[-- Type: text/plain, Size: 181 bytes --]

2011-09-13  Abhijit Halder  <abhijit.k.halder@gmail.com>

	Fix PR remote/10034:
	* cli/cli-setshow.c (do_setshow_command): Clear trailing whitespace
	from command argument strings.

[-- Attachment #3: gdb-space-issue.patch --]
[-- Type: text/x-patch, Size: 1801 bytes --]

Index: gdb/cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.46
diff -a -p -u -r1.46 cli-setshow.c
--- gdb/cli/cli-setshow.c	4 Aug 2011 19:10:13 -0000	1.46
+++ gdb/cli/cli-setshow.c	26 Sep 2011 05:36:01 -0000
@@ -177,15 +177,18 @@ do_setshow_command (char *arg, int from_
 	  }
 	  break;
 	case var_string_noescape:
-	  if (arg == NULL)
-	    arg = "";
-	  if (*(char **) c->var != NULL)
-	    xfree (*(char **) c->var);
-	  *(char **) c->var = xstrdup (arg);
-	  break;
 	case var_optional_filename:
 	  if (arg == NULL)
 	    arg = "";
+          else
+            {
+              /* Clear trailing whitespace.  */
+              char *ptr = arg + strlen (arg) - 1;
+
+              while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
+                ptr--;
+              *(ptr + 1) = '\0';
+            }
 	  if (*(char **) c->var != NULL)
 	    xfree (*(char **) c->var);
 	  *(char **) c->var = xstrdup (arg);
@@ -193,16 +196,17 @@ do_setshow_command (char *arg, int from_
 	case var_filename:
 	  if (arg == NULL)
 	    error_no_arg (_("filename to set it to."));
+          else
+            {
+              /* Clear trailing whitespace.  */
+              char *ptr = arg + strlen (arg) - 1;
+
+              while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
+                ptr--;
+              *(ptr + 1) = '\0';
+            }
 	  if (*(char **) c->var != NULL)
 	    xfree (*(char **) c->var);
-	  {
-	    /* Clear trailing whitespace of filename.  */
-	    char *ptr = arg + strlen (arg) - 1;
-
-	    while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
-	      ptr--;
-	    *(ptr + 1) = '\0';
-	  }
 	  *(char **) c->var = tilde_expand (arg);
 	  break;
 	case var_boolean: