cmd: an issue with cd .. command

[Nowres Rafid]

Hello,

the command "cd ..   " doesn't work when there is extra spaces at the
end.

I propose this patch and ask for your confirmation about it.

best regards.

[Jerome Leclanche]

You need to write tests for as many cases you can think of. eg cd .,
cd .., some invalid directories and so on. Im not familiar with the
cmd code but this feels like a bit of a hack.

Jerome

On Thu, Aug 11, 2011 at 6:19 AM, Nowres Rafid  wrote:
> Hello,
>
> the command "cd ..   " doesn't work when there is extra spaces at the end.
>
> I propose this patch and ask for your confirmation about it.
>
> best regards.

[Frédéric Delanoy]

On Thu, Aug 11, 2011 at 05:19, Nowres Rafid  wrote:
> Hello,
>
> the command "cd ..   " doesn't work when there is extra spaces at the end.
>
> I propose this patch and ask for your confirmation about it.
>
> best regards.

}
   else {
+
+    /* Remove any space at the end of command
+        This was because "cd .. " wasn't working */
+      {
+        WCHAR *ptr = command + strlenW(command) - 1;
+
+        while(*ptr == ' ')
+            *ptr-- = 0;
+      }

You shouldn't modify the "command" parameter. This one ought to be
constant, even if it's not currently marked so.
It's better to change the "string" string.

Also you should add testcases to cmd's test suite, possibly as a
preliminary patch of a series, but it can also be together with the
patch. That's up to you.

Also for single patches, you don't need "[1/1]"... use git
format-patch "-k" option, or edit the subject before submission.

Frédéric

[Nowres Rafid]

I corrected the code as suggested
and added some test cases

On Thu, 2011-08-11 at 14:03 +0200, Frédéric Delanoy wrote:
> On Thu, Aug 11, 2011 at 05:19, Nowres Rafid  wrote:
> > Hello,
> >
> > the command "cd ..   " doesn't work when there is extra spaces at the end.
> >
> > I propose this patch and ask for your confirmation about it.
> >
> > best regards.
>
> }
>    else {
> -
> - /* Remove any space at the end of command
> -
>    This was because "cd .. " wasn't working */
> -
>  {
> -
>    WCHAR *ptr = command + strlenW(command) - 1;
> -
> -
>    while(*ptr == ' ')
> -
>        *ptr-- = 0;
> -
>  }
>
> You shouldn't modify the "command" parameter. This one ought to be
> constant, even if it's not currently marked so.
> It's better to change the "string" string.
>
> Also you should add testcases to cmd's test suite, possibly as a
> preliminary patch of a series, but it can also be together with the
> patch. That's up to you.
>
> Also for single patches, you don't need "[1/1]"... use git
> format-patch "-k" option, or edit the subject before submission.
>
> Frédéric