Re: cmd: Use WCMD_output_asis*() for strings that are not supposed to contain formating directives.

[Frédéric Delanoy]

On Thu, Nov 24, 2011 at 00:30, Francois Gouget  wrote:
> Using WCMD_output*() instead could cause crashes.
>
> For instance if a filename is '%s'.
> This patch should not interfere with the previous one.
> - WCMD_output(newline);
> - WCMD_output_asis(newline);

...
> static const WCHAR newLine2[] = {'\n','\n','\0'};
> - if (!bare) WCMD_output (newLine2);
> - if (!bare) WCMD_output_asis (newLine2);

...
> - WCMD_output(anykey);
> - WCMD_output_asis(anykey);

No need to change those, since we control those strings, and those are
generic strings with no '%' directive.
> - WCMD_output (version_string);
> - WCMD_output_asis (version_string);

This one does contain a %s ( WCMD_VERSION,"CMD Version %s\n"), but we
control the version string as well (currently Wine version), so
shouldn't probably be changed.
> @@ -430,13 +430,13 @@ static DIRECTORY_STACK *WCMD_list_directory (DIRECTORY_STACK *inputparms, int le
>                 strcatW(&temp[toWrite], space);
>                 toWrite++;
>                 if (toWrite > 99) {
> - WCMD_output(temp);
> - WCMD_output_asis(temp);
>
> toWrite = 0;
>                     strcpyW(temp, nullW);
>                 }
>                 padding--;
>             }
> - WCMD_output(temp);
> - WCMD_output_asis(temp);
>
> }

It's just doing padding with plain spaces, so no changes needed here.

The remaining hunks should be OK.

Frédéric

cmd: Use WCMD_output_asis*() for strings that are not supposed to contain formating directives.

[Francois Gouget]

On Thu, 24 Nov 2011, Frédéric Delanoy wrote:
[...]
> > - WCMD_output(newline);
> > - WCMD_output_asis(newline);
>
> ...
> > static const WCHAR newLine2[] = {'\n','\n','\0'};
> > - if (!bare) WCMD_output (newLine2);
> > - if (!bare) WCMD_output_asis (newLine2);
>
> ...
> > - WCMD_output(anykey);
> > - WCMD_output_asis(anykey);
>
> No need to change those, since we control those strings, and those are
> generic strings with no '%' directive.

Sure these won't cause a crash if left as is.

But as you said we do control these strings and thus we know they don't 
need to go through wsprintf() and an extra string buffer before being 
passed on to WCMD_output_asis_*().

Also changing them to WCMD_output_asis_*() means fewer WCMD_output*() 
calls to audit for potential issues (not just '%' directives but also 
buffer size issues, etc).

So I think it's still very worthwhile to change them.

cmd: Use WCMD_output_asis*() for strings that are not supposed to contain formating directives.

[Frédéric Delanoy]

2011/11/24 Francois Gouget :
> On Thu, 24 Nov 2011, Frédéric Delanoy wrote:
> [...]
> > > - WCMD_output(newline);
> > > - WCMD_output_asis(newline);
> >
> > ...
> > > static const WCHAR newLine2[] = {'\n','\n','\0'};
> > > - if (!bare) WCMD_output (newLine2);
> > > - if (!bare) WCMD_output_asis (newLine2);
> >
> > ...
> > > - WCMD_output(anykey);
> > > - WCMD_output_asis(anykey);
> >
> > No need to change those, since we control those strings, and those are
> > generic strings with no '%' directive.
>
> Sure these won't cause a crash if left as is.
>
> But as you said we do control these strings and thus we know they don't
> need to go through wsprintf() and an extra string buffer before being
> passed on to WCMD_output_asis_*().
>
> Also changing them to WCMD_output_asis_*() means fewer WCMD_output*()
> calls to audit for potential issues (not just '%' directives but also
> buffer size issues, etc).
>
> So I think it's still very worthwhile to change them.

Right. You have a point. This patch may go in as is in that case.

Frédéric