Re: 2/2 start.exe: Handle the process title argument (try3)
Alexander Nicolaysen Sørnes <[EMAIL PROTECTED]> writes:
> + vi.dwOSVersionInfoSize = sizeof(vi);
> + GetVersionExW(&vi);
> +
> + /* On Windows 9x start does not have the process title argument.
> +It works such that if the first parameter begins with a double quote
> ("),
> +it is taken as the process title. Thus 'start "notepad" a' will
> work on
> +Windows 9x but not NT variants */
> + if(argc > 1 && (vi.dwMajorVersion >= 5 || (vi.dwMajorVersion == 4 &&
> vi.dwPlatformId == 2)))
The right way to check for Win9x is GetVersion() & 0x8000.
> + {
> + int cmdcount;
> + WCHAR* cmdline = GetCommandLineW();
> + WCHAR** cmdargs = CommandLineToArgvW(cmdline, &cmdcount);
> + WCHAR* pos = cmdline;
> +
> + pos += lstrlenW(cmdargs[0]);
This won't work if the program name is quoted.
--
Alexandre Julliard
[EMAIL PROTECTED]
Re: 2/2 start.exe: Handle the process title argument (try3)
On Wednesday 30 January 2008 04:52:33 Dmitry Timoshkov wrote:
> "Alexander Nicolaysen Sørnes" <[EMAIL PROTECTED]> wrote:
> > try 2 (thanks to Dmitry & Detlef)
> >
> > - Use GlobalFree instead of LocalFree
> > - Fix indentation
> > - Remove part of a comment
> >
> > try 3
> > - Use LocalFree, fixing CommandLineToargvW first
>
> The following question from my original review still has not been answered:
>
> wmain() already has arc/argv pair of parameters, why do you need to parse
> command line again?
We need to check for the first parameter beginning with a double quote (").
This is stripped from the argv array.
Alexander
Re: 2/2 start.exe: Handle the process title argument (try3)
"Alexander Nicolaysen Sørnes" <[EMAIL PROTECTED]> wrote: > try 2 (thanks to Dmitry & Detlef) > > - Use GlobalFree instead of LocalFree > - Fix indentation > - Remove part of a comment > > try 3 > - Use LocalFree, fixing CommandLineToargvW first The following question from my original review still has not been answered: wmain() already has arc/argv pair of parameters, why do you need to parse command line again? -- Dmitry.
Re: start.exe: Handle the process title argument (try2)
On Tuesday 29 January 2008 04:10:34 Dmitry Timoshkov wrote: > "Alexander Nicolaysen Sørnes" <[EMAIL PROTECTED]> wrote: > > - Use GlobalFree instead of LocalFree > > According to MSDN LocalFree is the correct one, as I asked before > you need to fix Wine implementation of CommandLineToArgvW. Sorry, I misread your previous post.THanks for point it out. Alexander
Re: start.exe: Handle the process title argument (try2)
"Alexander Nicolaysen Sørnes" <[EMAIL PROTECTED]> wrote: > - Use GlobalFree instead of LocalFree According to MSDN LocalFree is the correct one, as I asked before you need to fix Wine implementation of CommandLineToArgvW. -- Dmitry.
Re: start.exe: Handle the process title argument
On Monday 28 January 2008 04:58:12 you wrote:
> "Alexander Nicolaysen Sørnes" <[EMAIL PROTECTED]> wrote:
> > + NOTE: This will only work when run from Wine's cmd, but
> > that's ok as start is a builtin + shell command on NT */
>
> What's the purpose of the comment above?
>
Just to be informative to someone working with start.exe. I can remove it.
> > +if(argc > 1 && (vi.dwMajorVersion >= 5 || (vi.dwMajorVersion ==
> > 4 && vi.dwPlatformId == 2)))
>
> Unless there is an app which would break because of the title set there is
> no need to check the version.
>
I don't have an app handy that relies on the DOS/Win 9x behaviour of start,
but commands like
start "notepad" a.txt
start "C:\Program Files\app.exe"
would work with the old behaviour but not with the new one; thus I assumed
some apps would break if not checking the Windows version.
> > +{
> > +int cmdcount;
> > +WCHAR* cmdline = GetCommandLineW();
> > +WCHAR** cmdargs = CommandLineToArgvW(cmdline, &cmdcount);
> > +WCHAR* pos = cmdline;
>
> wmain() already has arc/argv pair of parameters, why do you need to parse
> command line again?
>
We need to check for the first parameter beginning with a double quote (").
This is stripped from the argv array.
> > +
> > +pos += lstrlenW(cmdargs[0]);
> > +
> > +while(*pos == ' ')
> > +pos++;
> > +
> > +if(*pos == '"')
> > +{
> > +/* FIXME: Set the process title */
> > +i++;
> > +}
> > +LocalFree(cmdargs);
> > +}
>
> I assume that you've added LocalFree as MSDN suggests, but
> CommandLineToArgvW in Wine uses GlobalAlloc, perhaps you could fix that?
Sure. :)
Re: start.exe: Handle the process title argument
On So, 2008-01-27 at 21:18 +0100, Alexander Nicolaysen Sørnes wrote: > SHELLEXECUTEINFOW sei; > +OSVERSIONINFOW vi; > WCHAR *args = NULL; > - int i; > + int i = 1; > The idention show a mismatch between SPACE and TAB -- By by ... Detlef
Re: start.exe: Handle the process title argument
"Alexander Nicolaysen Sørnes" <[EMAIL PROTECTED]> wrote:
> + NOTE: This will only work when run from Wine's cmd, but that's ok
> as start is a builtin
> + shell command on NT */
What's the purpose of the comment above?
> +if(argc > 1 && (vi.dwMajorVersion >= 5 || (vi.dwMajorVersion == 4 &&
> vi.dwPlatformId == 2)))
Unless there is an app which would break because of the title set there is no
need to check the version.
> +{
> +int cmdcount;
> +WCHAR* cmdline = GetCommandLineW();
> +WCHAR** cmdargs = CommandLineToArgvW(cmdline, &cmdcount);
> +WCHAR* pos = cmdline;
wmain() already has arc/argv pair of parameters, why do you need to parse
command line again?
> +
> +pos += lstrlenW(cmdargs[0]);
> +
> +while(*pos == ' ')
> +pos++;
> +
> +if(*pos == '"')
> +{
> +/* FIXME: Set the process title */
> +i++;
> +}
> +LocalFree(cmdargs);
> +}
I assume that you've added LocalFree as MSDN suggests, but CommandLineToArgvW
in Wine uses GlobalAlloc, perhaps you could fix that?
--
Dmitry.
