Re: cmd: Added support for "&&" and "||" condition checking.

[Alexandre Julliard]

Alexander Coffin <alexcoffin1999(a)gmail.com> writes:
> @@ -1478,7 +1557,9 @@ void WCMD_execute (const WCHAR *command, const WCHAR *redirects,
>      switch (i) {
>
> case WCMD_CALL:
> +        push_errorlevel_changed();
>          WCMD_call (p);
> +        pop_errorlevel_changed();

This seems to be the only place that does this, so I don't see why you
need to manage your own stack instead of saving/restoring things on the
process stack.

Also returning a success/failure value from the various functions that
implement commands would probably be cleaner than adding a global
'changed' flag.

-- 
Alexandre Julliard
julliard(a)winehq.org

cmd: Added support for "&&" and "||" condition checking.

[Alex C]

I have to agree that that isn't the most efficient method for keeping
track of the "errorlevel_changed". I will be resubmitting the patch
with a better implementation, using the process stack as you
suggested.

However, I would like to say that it really wouldn't be possible to only
use the success/failure value from the functions that don't change the
"errorlevel". This is because there is a subtle difference that sadly
Windows thought was a great idea (I think it should work like you say).
Windows checks if the "errorlevel" has changed in *any* of the commands
inside of a call and if it has then it checks the errorlevel, if it hasn't
changed it assumes a success. This means that even if the last
command "succeeds" without changing the errorlevel it doesn't matter
(e.g. "echo test") if any of the previous commands inside of a call
updated the errorlevel. Working examples can be seen on line 291-312
of programs/cmd/tests/test-builtins.cmd or a small snippet of them
below:

-snip
> :cfail
> echo %1
> call :setError 1
> goto :eof
> -snip-
> call :cfail c1||call :cfail c2
> -snip-
> :setError
> exit /B %1
> -snip-

The code above will return (and would do so even if one inserted
a "echo foobar" after the "call :setError 1"):
c1
c2


On Tue, Sep 19, 2017 at 5:27 AM, Alexandre Julliard <julliard(a)winehq.org>
wrote:
> Alexander Coffin <alexcoffin1999(a)gmail.com> writes:
> > @@ -1478,7 +1557,9 @@ void WCMD_execute (const WCHAR *command, const
> > WCHAR *redirects,
> >      switch (i) {
> >
> > case WCMD_CALL:
> > +        push_errorlevel_changed();
> >          WCMD_call (p);
> > +        pop_errorlevel_changed();
>
> This seems to be the only place that does this, so I don't see why you
> need to manage your own stack instead of saving/restoring things on the
> process stack.
>
> Also returning a success/failure value from the various functions that
> implement commands would probably be cleaner than adding a global
> 'changed' flag.
>
> --
> Alexandre Julliard
> julliard(a)winehq.org