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

[Jacek Caban]

Hi Alexander,

On 06.11.2017 02:51, Alexander Coffin wrote:
> Fixes: https://bugs.winehq.org/show_bug.cgi?id=32679
>
> This version (v3) is now completely c89 compliant. The previous
> had a one line issue that was not.
>
> Before "&&" and "||" were supported, but they also didn't check if past
> commands had failed. This commit removes direct access to errorlevel.
> Now errorlevel can only be accessed using getters and setters, but the
> it seemed like the most efficient solution. Also please note I added one
> TODO task. Currently this commit doesn't replicate a certain windows
> bug with setting errorlevel to 1 in some cases. See test_builtins.cmd
> for more details.
>
> I would have checked the success/failure of commands that don't set the
> errorlevel if that was possible, but Winows doesn't do that. It doesn't
> care about the most recent statement, it only cares about the
> errorlevel if it changes, otherwise it assumes success (if the
> errorlevel doesn't change).

I must admit that depending on the fact if errorlevel changed looks
questionable to me. You probably mostly want to handle cases of
successes that don't end up clearing errorlevel, but there are other
ways to do that. While looking for info about this, I found a very nice
answer on Stackoverflow:

https://stackoverflow.com/questions/34936240/batch-goto-loses-errorlevel/349...

What I like about this is that it covers everything with tests (although
I didn't verify results myself). It seems to prove that the actual
success of commands is separated from errorlevel and should be handled
separately. I think it would make sense to preserve actual command
result and handle it separately from errorlevel. Then we could use that
to decide on failure/success in || and && operators.

Implementing that is a bit more work than this patch. I think we'd want
WCMD_execute to return int representing executed command return code
(that means that WCMD_if, WCMD_for, WCMD_move, ECMD_run_program and
alike will also have to do it). Once you have that, you can simply use
returned value to properly handle && and || operators. How does that
sound to you?

Thanks,
Jacek