[PATCH v4 0/2] MR3671: cmd: refactored setting of 'errorlevel'
Encapsulate setting of 'errorlevel' in a separate function as the first stage of fixing bug #32679 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=32679 -- v4: cmd: fixed command chaining https://gitlab.winehq.org/wine/wine/-/merge_requests/3671
From: Dmitry Sokolov <mr.dmitry.sokolov(a)gmail.com> Encapsulate setting of 'errorlevel' in a separate function as the first stage of fixing bug #32679 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=32679 --- programs/cmd/batch.c | 2 +- programs/cmd/builtins.c | 80 ++++++++++++++++++++-------------------- programs/cmd/directory.c | 18 ++++----- programs/cmd/wcmd.h | 5 +++ programs/cmd/wcmdmain.c | 13 +++++-- 5 files changed, 65 insertions(+), 53 deletions(-) diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 3586cd45731..dcde0899b0f 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -653,7 +653,7 @@ void WCMD_call (WCHAR *command) { if (*command != ':') { WCMD_run_program(command, TRUE); /* If the thing we try to run does not exist, call returns 1 */ - if (errorlevel) errorlevel=1; + if (errorlevel) WCMD_set_errorlevel(1); } else { WCHAR gotoLabel[MAX_PATH]; diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 419e812103a..a0cb7025d83 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -268,7 +268,7 @@ void WCMD_choice (const WCHAR * args) { BOOL opt_s = FALSE; have_console = GetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), &oldmode); - errorlevel = 0; + WCMD_set_errorlevel(0); my_command = xstrdupW(WCMD_skip_leading_spaces((WCHAR*)args)); @@ -387,7 +387,7 @@ void WCMD_choice (const WCHAR * args) { if (!WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, 1, &count)) { free(my_command); - errorlevel = 0; + WCMD_set_errorlevel(0); return; } @@ -401,7 +401,7 @@ void WCMD_choice (const WCHAR * args) { if (have_console) SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), oldmode); - errorlevel = (ptr - opt_c) + 1; + WCMD_set_errorlevel((ptr - opt_c) + 1); WINE_TRACE("answer: %ld\n", errorlevel); free(my_command); return; @@ -606,12 +606,12 @@ void WCMD_copy(WCHAR * args) { COPY_FILES *prevcopy = NULL; /* Assume we were successful! */ - errorlevel = 0; + WCMD_set_errorlevel(0); /* If no args supplied at all, report an error */ if (param1[0] == 0x00) { WCMD_output_stderr (WCMD_LoadMessage(WCMD_NOARG)); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } @@ -691,7 +691,7 @@ void WCMD_copy(WCHAR * args) { if (*thisparam=='+') { if (lastcopyentry == NULL) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_SYNTAXERR)); - errorlevel = 1; + WCMD_set_errorlevel(1); goto exitreturn; } else { concatnextfilename = TRUE; @@ -746,7 +746,7 @@ void WCMD_copy(WCHAR * args) { } else { /* We have processed sources and destinations and still found more to do - invalid */ WCMD_output_stderr(WCMD_LoadMessage(WCMD_SYNTAXERR)); - errorlevel = 1; + WCMD_set_errorlevel(1); goto exitreturn; } concatnextfilename = FALSE; @@ -763,7 +763,7 @@ void WCMD_copy(WCHAR * args) { /* Ensure we have at least one source file */ if (!sourcelist) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_SYNTAXERR)); - errorlevel = 1; + WCMD_set_errorlevel(1); goto exitreturn; } @@ -1011,7 +1011,7 @@ void WCMD_copy(WCHAR * args) { } if (!status) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } else { WINE_TRACE("Copied successfully\n"); if (anyconcats) writtenoneconcat = TRUE; @@ -1023,7 +1023,7 @@ void WCMD_copy(WCHAR * args) { if (!destination->binarycopy && !anyconcats && !thiscopy->binarycopy) { if (!WCMD_AppendEOF(outname)) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } } } @@ -1035,7 +1035,7 @@ void WCMD_copy(WCHAR * args) { /* Error if the first file was not found */ if (!anyconcats || !writtenoneconcat) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } } @@ -1047,7 +1047,7 @@ void WCMD_copy(WCHAR * args) { if (!errorlevel && !destination->binarycopy && anyconcats && writtenoneconcat) { if (!WCMD_AppendEOF(destination->name)) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } } @@ -1130,7 +1130,7 @@ void WCMD_create_dir (WCHAR *args) { if (!argN) break; if (!create_full_path(thisArg)) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } } } @@ -1417,7 +1417,7 @@ BOOL WCMD_delete (WCHAR *args) { BOOL argsProcessed = FALSE; BOOL foundAny = FALSE; - errorlevel = 0; + WCMD_set_errorlevel(0); for (argno=0; ; argno++) { BOOL found; @@ -2408,7 +2408,7 @@ void WCMD_for (WCHAR *p, CMD_LIST **cmdList) { if (!input) { WCMD_print_error (); WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), item); - errorlevel = 1; + WCMD_set_errorlevel(1); return; /* FOR loop aborts at first failure here */ } else { @@ -2695,7 +2695,7 @@ void WCMD_pushd (const WCHAR *args) /* Change directory using CD code with /D parameter */ lstrcpyW(quals, L"/D"); GetCurrentDirectoryW (1024, thisdir); - errorlevel = 0; + WCMD_set_errorlevel(0); WCMD_setshow_default(args); if (errorlevel) { LocalFree(curdir); @@ -3055,7 +3055,7 @@ void WCMD_move (void) if (!status) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } } while (FindNextFileW(hff, &fd) != 0); @@ -3170,12 +3170,12 @@ void WCMD_rename (void) WCHAR fname[MAX_PATH]; WCHAR ext[MAX_PATH]; - errorlevel = 0; + WCMD_set_errorlevel(0); /* Must be at least two args */ if (param1[0] == 0x00 || param2[0] == 0x00) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOARG)); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } @@ -3183,7 +3183,7 @@ void WCMD_rename (void) if ((wcschr(param2,':') != NULL) || (wcschr(param2,'\\') != NULL)) { SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } @@ -3246,7 +3246,7 @@ void WCMD_rename (void) if (!status) { WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } } while (FindNextFileW(hff, &fd) != 0); @@ -3485,7 +3485,7 @@ void WCMD_setshow_default (const WCHAR *args) { status = SetCurrentDirectoryW(string); if (!status) { - errorlevel = 1; + WCMD_set_errorlevel(1); WCMD_print_error (); return; } else { @@ -4234,7 +4234,7 @@ void WCMD_setshow_env (WCHAR *s) { env = GetEnvironmentStringsW(); if (WCMD_setshow_sortenv( env, s ) == 0) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_MISSINGENV), s); - errorlevel = 1; + WCMD_set_errorlevel(1); } return; } @@ -4246,9 +4246,9 @@ void WCMD_setshow_env (WCHAR *s) { status = SetEnvironmentVariableW(s, p); gle = GetLastError(); if ((!status) & (gle == ERROR_ENVVAR_NOT_FOUND)) { - errorlevel = 1; + WCMD_set_errorlevel(1); } else if (!status) WCMD_print_error(); - else if (!interactive) errorlevel = 0; + else if (!interactive) WCMD_set_errorlevel(0); } } @@ -4474,9 +4474,11 @@ void WCMD_start(WCHAR *args) if (CreateProcessW( file, cmdline, NULL, NULL, TRUE, 0, NULL, NULL, &st, &pi )) { + DWORD exitCode = 0; WaitForSingleObject( pi.hProcess, INFINITE ); - GetExitCodeProcess( pi.hProcess, &errorlevel ); - if (errorlevel == STILL_ACTIVE) errorlevel = 0; + GetExitCodeProcess( pi.hProcess, &exitCode ); + if (exitCode == STILL_ACTIVE) WCMD_set_errorlevel(0); + else WCMD_set_errorlevel(exitCode); CloseHandle(pi.hProcess); CloseHandle(pi.hThread); } @@ -4484,7 +4486,7 @@ void WCMD_start(WCHAR *args) { SetLastError(ERROR_FILE_NOT_FOUND); WCMD_print_error (); - errorlevel = 9009; + WCMD_set_errorlevel(9009); } free(cmdline); } @@ -4518,7 +4520,7 @@ void WCMD_type (WCHAR *args) { if (param2[0] != 0x00) writeHeaders = TRUE; /* Loop through all args */ - errorlevel = 0; + WCMD_set_errorlevel(0); while (argN) { WCHAR *thisArg = WCMD_parameter (args, argno++, &argN, FALSE, FALSE); @@ -4534,7 +4536,7 @@ void WCMD_type (WCHAR *args) { if (h == INVALID_HANDLE_VALUE) { WCMD_print_error (); WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); - errorlevel = 1; + WCMD_set_errorlevel(1); } else { if (writeHeaders) { WCMD_output_stderr(L"\n%1\n\n\n", thisArg); @@ -4565,7 +4567,7 @@ void WCMD_more (WCHAR *args) { DWORD count; /* Prefix the NLS more with '-- ', then load the text */ - errorlevel = 0; + WCMD_set_errorlevel(0); lstrcpyW(moreStr, L"-- "); LoadStringW(hinst, WCMD_MORESTR, &moreStr[3], ARRAY_SIZE(moreStr)-3); @@ -4629,7 +4631,7 @@ void WCMD_more (WCHAR *args) { if (h == INVALID_HANDLE_VALUE) { WCMD_print_error (); WCMD_output_stderr(WCMD_LoadMessage(WCMD_READFAIL), thisArg); - errorlevel = 1; + WCMD_set_errorlevel(1); } else { ULONG64 curPos = 0; ULONG64 fileLen = 0; @@ -4771,7 +4773,7 @@ void WCMD_exit (CMD_LIST **cmdList) { int rc = wcstol(param1, NULL, 10); /* Note: wcstol of empty parameter is 0 */ if (context && lstrcmpiW(quals, L"/B") == 0) { - errorlevel = rc; + WCMD_set_errorlevel(rc); context -> skip_rest = TRUE; *cmdList = NULL; } else { @@ -4797,7 +4799,7 @@ void WCMD_assoc (const WCHAR *args, BOOL assoc) { HKEY readKey; /* See if parameter includes '=' */ - errorlevel = 0; + WCMD_set_errorlevel(0); newValue = wcschr(args, '='); if (newValue) accessOptions |= KEY_WRITE; @@ -4885,7 +4887,7 @@ void WCMD_assoc (const WCHAR *args, BOOL assoc) { LoadStringW(hinst, WCMD_NOFTYPE, msgbuffer, ARRAY_SIZE(msgbuffer)); } WCMD_output_stderr(msgbuffer, keyValue); - errorlevel = 2; + WCMD_set_errorlevel(2); } /* Not a query - it's a set or clear of a value */ @@ -4910,7 +4912,7 @@ void WCMD_assoc (const WCHAR *args, BOOL assoc) { } else if (assoc && rc != ERROR_FILE_NOT_FOUND) { WCMD_print_error(); - errorlevel = 2; + WCMD_set_errorlevel(2); } else { WCHAR msgbuffer[MAXSTRING]; @@ -4922,7 +4924,7 @@ void WCMD_assoc (const WCHAR *args, BOOL assoc) { LoadStringW(hinst, WCMD_NOFTYPE, msgbuffer, ARRAY_SIZE(msgbuffer)); } WCMD_output_stderr(msgbuffer, args); - errorlevel = 2; + WCMD_set_errorlevel(2); } /* It really is a set value = contents */ @@ -4938,7 +4940,7 @@ void WCMD_assoc (const WCHAR *args, BOOL assoc) { if (rc != ERROR_SUCCESS) { WCMD_print_error(); - errorlevel = 2; + WCMD_set_errorlevel(2); } else { WCMD_output_asis(args); WCMD_output_asis(L"="); @@ -4989,7 +4991,7 @@ void WCMD_color (void) { /* Fail if fg == bg color */ if (((color & 0xF0) >> 4) == (color & 0x0F)) { - errorlevel = 1; + WCMD_set_errorlevel(1); return; } diff --git a/programs/cmd/directory.c b/programs/cmd/directory.c index 6efcbb3a841..10f2d36b6b1 100644 --- a/programs/cmd/directory.c +++ b/programs/cmd/directory.c @@ -271,7 +271,7 @@ static DIRECTORY_STACK *WCMD_list_directory (DIRECTORY_STACK *inputparms, int le if (fd == NULL) { FindClose (hff); WINE_ERR("Out of memory\n"); - errorlevel = 1; + WCMD_set_errorlevel(1); return parms->next; } } while (FindNextFileW(hff, &fd[entry_count]) != 0); @@ -494,7 +494,7 @@ static DIRECTORY_STACK *WCMD_list_directory (DIRECTORY_STACK *inputparms, int le if ((file_total + dir_total == 0) && (level == 0)) { SetLastError (ERROR_FILE_NOT_FOUND); WCMD_print_error (); - errorlevel = 1; + WCMD_set_errorlevel(1); } return parms; @@ -549,7 +549,7 @@ void WCMD_directory (WCHAR *args) WCHAR fname[MAX_PATH]; WCHAR ext[MAX_PATH]; - errorlevel = 0; + WCMD_set_errorlevel(0); /* Prefill quals with (uppercased) DIRCMD env var */ if (GetEnvironmentVariableW(L"DIRCMD", string, ARRAY_SIZE(string))) { @@ -635,7 +635,7 @@ void WCMD_directory (WCHAR *args) } else { SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } break; @@ -655,7 +655,7 @@ void WCMD_directory (WCHAR *args) default: SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } p++; @@ -686,7 +686,7 @@ void WCMD_directory (WCHAR *args) default: SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } @@ -705,7 +705,7 @@ void WCMD_directory (WCHAR *args) default: SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); - errorlevel = 1; + WCMD_set_errorlevel(1); return; } p = p + 1; @@ -840,7 +840,7 @@ void WCMD_directory (WCHAR *args) status = WCMD_volume (0, drive); trailerReqd = TRUE; if (!status) { - errorlevel = 1; + WCMD_set_errorlevel(1); goto exit; } } @@ -849,7 +849,7 @@ void WCMD_directory (WCHAR *args) } /* Clear any errors from previous invocations, and process it */ - errorlevel = 0; + WCMD_set_errorlevel(0); prevEntry = thisEntry; thisEntry = WCMD_list_directory (thisEntry, 0); } diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 1935bbcdcd2..0710a57c827 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -204,6 +204,11 @@ extern BATCH_CONTEXT *context; extern FOR_CONTEXT forloopcontext; extern BOOL delayedsubst; +static inline void WCMD_set_errorlevel(DWORD newValue) +{ + errorlevel = newValue; +} + #endif /* !RC_INVOKED */ /* diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index d00c6e07566..513543f17d6 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -1046,7 +1046,7 @@ void WCMD_run_program (WCHAR *command, BOOL called) if (!firstParam) return; if (!firstParam[0]) { - errorlevel = 0; + WCMD_set_errorlevel(0); return; } @@ -1244,8 +1244,13 @@ void WCMD_run_program (WCHAR *command, BOOL called) or for console applications */ if (!interactive || (console && !HIWORD(console))) WaitForSingleObject (pe.hProcess, INFINITE); - GetExitCodeProcess (pe.hProcess, &errorlevel); - if (errorlevel == STILL_ACTIVE) errorlevel = 0; + + { + DWORD exitCode = 0; + GetExitCodeProcess (pe.hProcess, &exitCode); + if (exitCode == STILL_ACTIVE) WCMD_set_errorlevel(0); + else WCMD_set_errorlevel(exitCode); + } CloseHandle(pe.hProcess); CloseHandle(pe.hThread); @@ -1271,7 +1276,7 @@ void WCMD_run_program (WCHAR *command, BOOL called) /* If a command fails to launch, it sets errorlevel 9009 - which does not seem to have any associated constant definition */ - errorlevel = 9009; + WCMD_set_errorlevel(9009); return; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3671
From: Dmitry Sokolov <mr.dmitry.sokolov(a)gmail.com> --- programs/cmd/builtins.c | 25 +++++++++++++++++++++++-- programs/cmd/wcmd.h | 7 +++++++ programs/cmd/wcmdmain.c | 37 ++++++++++++++++++++++++++++++++++--- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index a0cb7025d83..d6f23ae020c 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1485,6 +1485,8 @@ void WCMD_echo (const WCHAR *args) const WCHAR *origcommand = args; WCHAR *trimmed; + WCMD_set_lastCmdStatus(TRUE); + if ( args[0]==' ' || args[0]=='\t' || args[0]=='.' || args[0]==':' || args[0]==';' || args[0]=='/') args++; @@ -2723,6 +2725,8 @@ void WCMD_pushd (const WCHAR *args) void WCMD_popd (void) { struct env_stack *temp = pushd_directories; + WCMD_set_lastCmdStatus(FALSE); + if (!pushd_directories) return; @@ -2731,6 +2735,7 @@ void WCMD_popd (void) { SetCurrentDirectoryW(temp->strings); LocalFree (temp->strings); LocalFree (temp); + WCMD_set_lastCmdStatus(TRUE); } /******************************************************************* @@ -3084,6 +3089,8 @@ void WCMD_pause (void) WCMD_ReadFile(hIn, &key, 1, &count); if (have_console) SetConsoleMode(hIn, oldmode); + + WCMD_set_lastCmdStatus(TRUE); } /**************************************************************************** @@ -3098,6 +3105,8 @@ void WCMD_remove_dir (WCHAR *args) { int argsProcessed = 0; WCHAR *argN = args; + WCMD_set_lastCmdStatus(TRUE); + /* Loop through all args */ while (argN) { WCHAR *thisArg = WCMD_parameter (args, argno++, &argN, FALSE, FALSE); @@ -3109,7 +3118,10 @@ void WCMD_remove_dir (WCHAR *args) { /* If subdirectory search not supplied, just try to remove and report error if it fails (eg if it contains a file) */ if (wcsstr(quals, L"/S") == NULL) { - if (!RemoveDirectoryW(thisArg)) WCMD_print_error (); + if (!RemoveDirectoryW(thisArg)) { + WCMD_set_lastCmdStatus(FALSE); + WCMD_print_error(); + } /* Otherwise use ShFileOp to recursively remove a directory */ } else { @@ -3139,7 +3151,10 @@ void WCMD_remove_dir (WCHAR *args) { /* SHFileOperationW needs file list with a double null termination */ thisArg[lstrlenW(thisArg) + 1] = 0x00; - if (SHFileOperationW(&lpDir)) WCMD_print_error (); + if (SHFileOperationW(&lpDir)) { + WCMD_set_lastCmdStatus(FALSE); + WCMD_print_error (); + } } } } @@ -3350,6 +3365,8 @@ void WCMD_endlocal (void) { struct env_stack *temp; int len, n; + WCMD_set_lastCmdStatus(TRUE); + /* setlocal does nothing outside of batch programs */ if (!context) return; @@ -4346,6 +4363,8 @@ void WCMD_setshow_time (void) { void WCMD_shift (const WCHAR *args) { int start; + WCMD_set_lastCmdStatus(TRUE); + if (context != NULL) { WCHAR *pos = wcschr(args, '/'); int i; @@ -4355,6 +4374,7 @@ void WCMD_shift (const WCHAR *args) { } else if (*(pos+1)>='0' && *(pos+1)<='8') { start = (*(pos+1) - '0'); } else { + WCMD_set_lastCmdStatus(FALSE); SetLastError(ERROR_INVALID_PARAMETER); WCMD_print_error(); return; @@ -4498,6 +4518,7 @@ void WCMD_start(WCHAR *args) */ void WCMD_title (const WCHAR *args) { SetConsoleTitleW(args); + WCMD_set_lastCmdStatus(TRUE); } /**************************************************************************** diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 0710a57c827..a996071309f 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -200,6 +200,7 @@ typedef struct _FOR_CONTEXT { */ extern WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; extern DWORD errorlevel; +extern BOOL lastCmdStatus; extern BATCH_CONTEXT *context; extern FOR_CONTEXT forloopcontext; extern BOOL delayedsubst; @@ -207,6 +208,12 @@ extern BOOL delayedsubst; static inline void WCMD_set_errorlevel(DWORD newValue) { errorlevel = newValue; + lastCmdStatus = (errorlevel == 0); +} + +static inline void WCMD_set_lastCmdStatus(BOOL isSucceeded) +{ + lastCmdStatus = isSucceeded; } #endif /* !RC_INVOKED */ diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 513543f17d6..3ba1dba22b7 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -37,6 +37,7 @@ extern struct env_stack *pushd_directories; BATCH_CONTEXT *context = NULL; DWORD errorlevel; +BOOL lastCmdStatus; WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; BOOL interactive; FOR_CONTEXT forloopcontext; /* The 'for' loop context */ @@ -2365,6 +2366,7 @@ CMD_LIST *WCMD_process_commands(CMD_LIST *thisCmd, BOOL oneBracket, BOOL retrycall) { int bdepth = -1; + CMD_LIST *prevCmd = NULL; if (thisCmd && oneBracket) bdepth = thisCmd->bracketDepth; @@ -2386,12 +2388,41 @@ CMD_LIST *WCMD_process_commands(CMD_LIST *thisCmd, BOOL oneBracket, about them and it will be handled in there) Also, skip over any batch labels (eg. :fred) */ if (thisCmd->command && thisCmd->command[0] != ':') { - WINE_TRACE("Executing command: '%s'\n", wine_dbgstr_w(thisCmd->command)); - WCMD_execute (thisCmd->command, thisCmd->redirects, &thisCmd, retrycall); + /* Process the command chaining */ + if ( (thisCmd->prevDelim == CMD_ONSUCCESS && lastCmdStatus == FALSE) || + (thisCmd->prevDelim == CMD_ONFAILURE && lastCmdStatus == TRUE) ) { + if (prevCmd && prevCmd->bracketDepth < thisCmd->bracketDepth) { + /* Skipping the chain of commands in brackets */ + int bd = thisCmd->bracketDepth; + do { + WINE_TRACE("Skipping command '%s'\n", wine_dbgstr_w(thisCmd->command)); + prevCmd = thisCmd; + thisCmd = thisCmd->nextcommand; + } while (thisCmd && bd <= thisCmd->bracketDepth); + continue; + } else if (thisCmd->prevDelim == CMD_ONFAILURE && lastCmdStatus == TRUE) { + /* Skipping all chaining commands after '||', i.e. '|| cmd2 && cmd3 || cmd4'*/ + do { + WINE_TRACE("Skipping command '%s'\n", wine_dbgstr_w(thisCmd->command)); + prevCmd = thisCmd; + thisCmd = thisCmd->nextcommand; + } while (thisCmd && (thisCmd->prevDelim == CMD_ONSUCCESS || thisCmd->prevDelim == CMD_ONFAILURE)); + continue; + } else { + /* Skipping the next command */ + WINE_TRACE("Skipping command '%s'\n", wine_dbgstr_w(thisCmd->command)); + } + } else { + WINE_TRACE("Executing command: '%s'\n", wine_dbgstr_w(thisCmd->command)); + WCMD_execute (thisCmd->command, thisCmd->redirects, &thisCmd, retrycall); + } } /* Step on unless the command itself already stepped on */ - if (thisCmd == origCmd) thisCmd = thisCmd->nextcommand; + if (thisCmd == origCmd) { + prevCmd = thisCmd; + thisCmd = thisCmd->nextcommand; + } } return NULL; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3671
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=136876 Your paranoid android. === debian11 (32 bit report) === cmd.exe: batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 285 (got 'j3', wanted 'j3') batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 308 (got 'g3', wanted 'g3') batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 313 (got 'i3', wanted 'i3') batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 631 (got 'foo3 not created', wanted 'foo3 not created') === debian11b (64 bit WoW report) === cmd.exe: batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 285 (got 'j3', wanted 'j3') batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 308 (got 'g3', wanted 'g3') batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 313 (got 'i3', wanted 'i3') batch.c:321: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 631 (got 'foo3 not created', wanted 'foo3 not created')
another comment: * it looks a bit ackward to add the global variable lastCmdSucceeded... as it's just errorlevel == 0 * couldn't this be doable by just testing the value of errorlevel from previous command in chaining? * maybe it'll require first to ensure that all commands properly set errorlevel -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3671#note_43933
On Thu Aug 31 16:41:05 2023 +0000, eric pouech wrote:
another comment: * it looks a bit ackward to add the global variable lastCmdSucceeded... as it's just errorlevel == 0 * couldn't this be doable by just testing the value of errorlevel from previous command in chaining? * maybe it'll require first to ensure that all commands properly set errorlevel The ERRORLEVEL check is not enough. There should be a separate variable to hold the status of the last command.
https://ss64.com/nt/errorlevel.html
Even though a CMD batch script should set or reset ERRORLEVEL after every command, there are a few exceptions:
Commands that do NOT affect the ERRORLEVEL: \ BREAK, ECHO, ENDLOCAL, FOR, IF, PAUSE, REM, RD/RMDIR, TITLE
Commands that will set but not clear an ERRORLEVEL: \ CLS, GOTO, KEYS, POPD, SHIFT
Commands that set an Exit Code but not the ERRORLEVEL: \ RD/RMDIR
Commands that set an ERRORLEVEL but not the Exit Code ([SO explanation](https://stackoverflow.com/questions/34936240/batch-goto-loses-errorlevel/349...)): \ MD/MKDIR
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3671#note_43975
On Thu Aug 31 16:41:05 2023 +0000, Dmitry Sokolov wrote:
The ERRORLEVEL check is not enough. There should be a separate variable to hold the status of the last command. https://ss64.com/nt/errorlevel.html
Even though a CMD batch script should set or reset ERRORLEVEL after every command, there are a few exceptions:
Commands that do NOT affect the ERRORLEVEL: \ BREAK, ECHO, ENDLOCAL, FOR, IF, PAUSE, REM, RD/RMDIR, TITLE
Commands that will set but not clear an ERRORLEVEL: \ CLS, GOTO, KEYS, POPD, SHIFT
Commands that set an Exit Code but not the ERRORLEVEL: \ RD/RMDIR
Commands that set an ERRORLEVEL but not the Exit Code ([SO explanation](https://stackoverflow.com/questions/34936240/batch-goto-loses-errorlevel/349...)): \ MD/MKDIR in that case:
* do we have test cases that support these affirmations (from a cursory look, we have tests for errorlevel, but perhaps not with 100% coverage, but very few for exit code) * I don't like adding another global variable; it would be more natural for each command (built-in or external) to return its exit code and use it in chaining & redirection operators -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3671#note_44288
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3671
Likely superseded by the cmd.exe parser rewrite. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3671#note_86735
participants (5)
-
Alexandre Julliard (＠julliard) -
Dmitry Sokolov
-
Dmitry Sokolov (＠DmitrySokolov)
-
eric pouech (＠epo)
-
Marvin