From 4b1ae54046b6727652e8d2d98c297a6731711cc4 Mon Sep 17 00:00:00 2001 From: Serge Gautherie <32623169+SergeGautherie@users.noreply.github.com> Date: Tue, 26 May 2020 20:21:25 +0200 Subject: [PATCH] [WINSPOOL] GetPrinterDriverA(): Refactor failure handling (#2832) * [WINSPOOL] GetPrinterDriverA(): Refactor failure handling Addendum to d90beaeed26863985486874aaa2a1c3422fd6d9f. --- win32ss/printing/base/winspool/precomp.h | 2 +- win32ss/printing/base/winspool/printers.c | 351 +++++++++++++++------- win32ss/printing/base/winspool/utils.c | 29 +- 3 files changed, 253 insertions(+), 129 deletions(-) diff --git a/win32ss/printing/base/winspool/precomp.h b/win32ss/printing/base/winspool/precomp.h index 58eed626d02..e5a857b34e0 100644 --- a/win32ss/printing/base/winspool/precomp.h +++ b/win32ss/printing/base/winspool/precomp.h @@ -42,7 +42,7 @@ SPOOLER_HANDLE, *PSPOOLER_HANDLE; extern HANDLE hProcessHeap; // utils.c -extern BOOL UnicodeToAnsiInPlace(PWSTR pwszField); +DWORD UnicodeToAnsiInPlace(PWSTR pwszField); // devmode.c extern void RosConvertAnsiDevModeToUnicodeDevmode(PDEVMODEA pDevModeInput, PDEVMODEW pDevModeOutput); diff --git a/win32ss/printing/base/winspool/printers.c b/win32ss/printing/base/winspool/printers.c index 1576ca0edd4..249d41256bd 100644 --- a/win32ss/printing/base/winspool/printers.c +++ b/win32ss/printing/base/winspool/printers.c @@ -472,7 +472,6 @@ BOOL WINAPI EnumPrintersA(DWORD Flags, PSTR Name, DWORD Level, PBYTE pPrinterEnum, DWORD cbBuf, PDWORD pcbNeeded, PDWORD pcReturned) { DWORD dwErrorCode; - BOOL bResult; DWORD cch; PWSTR pwszName = NULL; PSTR pszPrinterName = NULL; @@ -525,8 +524,7 @@ EnumPrintersA(DWORD Flags, PSTR Name, DWORD Level, PBYTE pPrinterEnum, DWORD cbB } /* Ref: https://stackoverflow.com/questions/41147180/why-enumprintersa-and-enumprintersw-request-the-same-amount-of-memory */ - bResult = EnumPrintersW(Flags, pwszName, Level, pPrinterEnum, cbBuf, pcbNeeded, pcReturned); - if (!bResult) + if (!EnumPrintersW(Flags, pwszName, Level, pPrinterEnum, cbBuf, pcbNeeded, pcReturned)) { dwErrorCode = GetLastError(); goto Cleanup; @@ -1118,7 +1116,6 @@ BOOL WINAPI GetPrinterA(HANDLE hPrinter, DWORD Level, LPBYTE pPrinter, DWORD cbBuf, LPDWORD pcbNeeded) { DWORD dwErrorCode; - BOOL bResult; PPRINTER_INFO_1A ppi1a = (PPRINTER_INFO_1A)pPrinter; PPRINTER_INFO_1W ppi1w = (PPRINTER_INFO_1W)pPrinter; PPRINTER_INFO_2A ppi2a = (PPRINTER_INFO_2A)pPrinter; @@ -1141,8 +1138,7 @@ GetPrinterA(HANDLE hPrinter, DWORD Level, LPBYTE pPrinter, DWORD cbBuf, LPDWORD goto Cleanup; } - bResult = GetPrinterW(hPrinter, Level, pPrinter, cbBuf, pcbNeeded); - if (!bResult) + if (!GetPrinterW(hPrinter, Level, pPrinter, cbBuf, pcbNeeded)) { dwErrorCode = GetLastError(); goto Cleanup; @@ -1580,6 +1576,7 @@ Cleanup: BOOL WINAPI GetPrinterDriverA(HANDLE hPrinter, LPSTR pEnvironment, DWORD Level, LPBYTE pDriverInfo, DWORD cbBuf, LPDWORD pcbNeeded) { + DWORD dwErrorCode; /* * We are mapping multiple different pointers to the same pDriverInfo pointer here so that * we can use the same incoming pointer for different Levels @@ -1591,7 +1588,6 @@ GetPrinterDriverA(HANDLE hPrinter, LPSTR pEnvironment, DWORD Level, LPBYTE pDriv PDRIVER_INFO_5W pdi5w = (PDRIVER_INFO_5W)pDriverInfo; PDRIVER_INFO_6W pdi6w = (PDRIVER_INFO_6W)pDriverInfo; - BOOL bReturnValue = FALSE; DWORD cch; PWSTR pwszEnvironment = NULL; @@ -1600,9 +1596,9 @@ GetPrinterDriverA(HANDLE hPrinter, LPSTR pEnvironment, DWORD Level, LPBYTE pDriv // Check for invalid levels here for early error return. Should be 1-6. if (Level < 1 || Level > 6) { - SetLastError(ERROR_INVALID_LEVEL); + dwErrorCode = ERROR_INVALID_LEVEL; ERR("Invalid Level!\n"); - goto Exit; + goto Cleanup; } if (pEnvironment) @@ -1613,26 +1609,18 @@ GetPrinterDriverA(HANDLE hPrinter, LPSTR pEnvironment, DWORD Level, LPBYTE pDriv pwszEnvironment = HeapAlloc(hProcessHeap, 0, (cch + 1) * sizeof(WCHAR)); if (!pwszEnvironment) { - SetLastError(ERROR_NOT_ENOUGH_MEMORY); + dwErrorCode = ERROR_NOT_ENOUGH_MEMORY; ERR("HeapAlloc failed!\n"); - goto Exit; + goto Cleanup; } MultiByteToWideChar(CP_ACP, 0, pEnvironment, -1, pwszEnvironment, cch + 1); } - bReturnValue = GetPrinterDriverW(hPrinter, pwszEnvironment, Level, pDriverInfo, cbBuf, pcbNeeded); - TRACE("*pcbNeeded is '%d' and bReturnValue is '%d' and GetLastError is '%ld'.\n", *pcbNeeded, bReturnValue, GetLastError()); - - if (pwszEnvironment) - { - HeapFree(hProcessHeap, 0, pwszEnvironment); - } - - if (!bReturnValue) + if (!GetPrinterDriverW(hPrinter, pwszEnvironment, Level, pDriverInfo, cbBuf, pcbNeeded)) { - TRACE("GetPrinterDriverW failed!\n"); - goto Exit; + dwErrorCode = GetLastError(); + goto Cleanup; } // Do Unicode to ANSI conversions for strings based on Level @@ -1640,170 +1628,307 @@ GetPrinterDriverA(HANDLE hPrinter, LPSTR pEnvironment, DWORD Level, LPBYTE pDriv { case 1: { - if (!UnicodeToAnsiInPlace(pdi1w->pName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi1w->pName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } break; } case 2: { - if (!UnicodeToAnsiInPlace(pdi2w->pName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi2w->pName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi2w->pEnvironment)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi2w->pEnvironment); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi2w->pDriverPath)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi2w->pDriverPath); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi2w->pDataFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi2w->pDataFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi2w->pConfigFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi2w->pConfigFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } break; } case 3: { - if (!UnicodeToAnsiInPlace(pdi3w->pName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pEnvironment)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pEnvironment); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pDriverPath)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pDriverPath); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pDataFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pDataFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pConfigFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pConfigFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pHelpFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pHelpFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pDependentFiles)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pDependentFiles); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pMonitorName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pMonitorName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi3w->pDefaultDataType)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi3w->pDefaultDataType); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } break; } case 4: { - if (!UnicodeToAnsiInPlace(pdi4w->pName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pEnvironment)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pEnvironment); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pDriverPath)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pDriverPath); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pDataFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pDataFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pConfigFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pConfigFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pHelpFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pHelpFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pDependentFiles)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pDependentFiles); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pMonitorName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pMonitorName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pDefaultDataType)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pDefaultDataType); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi4w->pszzPreviousNames)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi4w->pszzPreviousNames); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } break; } case 5: { - if (!UnicodeToAnsiInPlace(pdi5w->pName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi5w->pName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi5w->pEnvironment)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi5w->pEnvironment); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi5w->pDriverPath)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi5w->pDriverPath); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi5w->pDataFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi5w->pDataFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi5w->pConfigFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi5w->pConfigFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } break; } case 6: { - if (!UnicodeToAnsiInPlace(pdi6w->pName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pEnvironment)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pEnvironment); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pDriverPath)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pDriverPath); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pDataFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pDataFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pConfigFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pConfigFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pHelpFile)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pHelpFile); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pDependentFiles)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pDependentFiles); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pMonitorName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pMonitorName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pDefaultDataType)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pDefaultDataType); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pszzPreviousNames)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pszzPreviousNames); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pszMfgName)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pszMfgName); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pszOEMUrl)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pszOEMUrl); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pszHardwareID)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pszHardwareID); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } - if (!UnicodeToAnsiInPlace(pdi6w->pszProvider)) - goto Exit; + dwErrorCode = UnicodeToAnsiInPlace(pdi6w->pszProvider); + if (dwErrorCode != ERROR_SUCCESS) + { + goto Cleanup; + } } } - bReturnValue = TRUE; + dwErrorCode = ERROR_SUCCESS; -Exit: +Cleanup: + if (pwszEnvironment) + { + HeapFree(hProcessHeap, 0, pwszEnvironment); + } - return bReturnValue; + SetLastError(dwErrorCode); + return (dwErrorCode == ERROR_SUCCESS); } BOOL WINAPI diff --git a/win32ss/printing/base/winspool/utils.c b/win32ss/printing/base/winspool/utils.c index ae190ed5108..f16cd95d014 100644 --- a/win32ss/printing/base/winspool/utils.c +++ b/win32ss/printing/base/winspool/utils.c @@ -7,16 +7,16 @@ #include "precomp.h" -BOOL UnicodeToAnsiInPlace(PWSTR pwszField) +/* + * Converts an incoming Unicode string to an ANSI string. + * It is only useful for "in-place" conversions where the ANSI string goes + * back into the same place where the Unicode string came into this function. + * + * It returns an error code. + */ +// TODO: It seems that many of the functions involving printing could use this. +DWORD UnicodeToAnsiInPlace(PWSTR pwszField) { - /* - * This converts an incoming Unicode string to an ANSI string. - * It returns FALSE on failure, otherwise it returns TRUE. - * It is only useful for "in-place" conversions where the ANSI string goes - * back into the same place where the Unicode string came into this function. - * It seems that many of the functions involving printing can use this. - */ - PSTR pszTemp; DWORD cch; @@ -29,21 +29,20 @@ BOOL UnicodeToAnsiInPlace(PWSTR pwszField) if (!pwszField) { - return TRUE; + return ERROR_SUCCESS; } cch = wcslen(pwszField); if (cch == 0) { - return TRUE; + return ERROR_SUCCESS; } pszTemp = HeapAlloc(hProcessHeap, 0, (cch + 1) * sizeof(CHAR)); - if (!pszField) + if (!pszTemp) { - SetLastError(ERROR_NOT_ENOUGH_MEMORY); ERR("HeapAlloc failed!\n"); - return FALSE; // indicates a failure to be handled by caller + return ERROR_NOT_ENOUGH_MEMORY; } WideCharToMultiByte(CP_ACP, 0, pwszField, -1, pszTemp, cch + 1, NULL, NULL); @@ -51,5 +50,5 @@ BOOL UnicodeToAnsiInPlace(PWSTR pwszField) HeapFree(hProcessHeap, 0, pszTemp); - return TRUE; + return ERROR_SUCCESS; } -- 2.17.1