diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index c01b1599b35..a0bcf71e38b 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -516,6 +516,9 @@ Trunk (Unreleased) HADOOP-9437. TestNativeIO#testRenameTo fails on Windows due to assumption that POSIX errno is embedded in NativeIOException. (Chris Nauroth via suresh) + + HADOOP-9443. Port winutils static code analysis change to trunk. + (Chuan Liu via suresh) Release 2.0.5-beta - UNRELEASED diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/chmod.c b/hadoop-common-project/hadoop-common/src/main/winutils/chmod.c index 98788bafdeb..c82e18e73e7 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/chmod.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/chmod.c @@ -74,18 +74,22 @@ static BOOL ParseMode(LPCWSTR modeString, PMODE_CHANGE_ACTION *actions); static BOOL FreeActions(PMODE_CHANGE_ACTION actions); -static BOOL ParseCommandLineArguments(__in int argc, __in wchar_t *argv[], - __out BOOL *rec, __out_opt INT *mask, - __out_opt PMODE_CHANGE_ACTION *actions, __out LPCWSTR *path); +static BOOL ParseCommandLineArguments( + __in int argc, + __in_ecount(argc) wchar_t *argv[], + __out BOOL *rec, + __out_opt INT *mask, + __out_opt PMODE_CHANGE_ACTION *actions, + __out LPCWSTR *path); static BOOL ChangeFileModeByActions(__in LPCWSTR path, - PMODE_CHANGE_ACTION actions); + MODE_CHANGE_ACTION const *actions); static BOOL ChangeFileMode(__in LPCWSTR path, __in_opt INT mode, - __in_opt PMODE_CHANGE_ACTION actions); + __in_opt MODE_CHANGE_ACTION const *actions); static BOOL ChangeFileModeRecursively(__in LPCWSTR path, __in_opt INT mode, - __in_opt PMODE_CHANGE_ACTION actions); + __in_opt MODE_CHANGE_ACTION const *actions); //---------------------------------------------------------------------------- @@ -99,7 +103,7 @@ static BOOL ChangeFileModeRecursively(__in LPCWSTR path, __in_opt INT mode, // // Notes: // -int Chmod(int argc, wchar_t *argv[]) +int Chmod(__in int argc, __in_ecount(argc) wchar_t *argv[]) { LPWSTR pathName = NULL; LPWSTR longPathName = NULL; @@ -169,7 +173,7 @@ ChmodEnd: // Notes: // static BOOL ChangeFileMode(__in LPCWSTR path, __in_opt INT unixAccessMask, - __in_opt PMODE_CHANGE_ACTION actions) + __in_opt MODE_CHANGE_ACTION const *actions) { if (actions != NULL) return ChangeFileModeByActions(path, actions); @@ -202,7 +206,7 @@ static BOOL ChangeFileMode(__in LPCWSTR path, __in_opt INT unixAccessMask, // - Otherwise, call the method on all its children, then change its mode. // static BOOL ChangeFileModeRecursively(__in LPCWSTR path, __in_opt INT mode, - __in_opt PMODE_CHANGE_ACTION actions) + __in_opt MODE_CHANGE_ACTION const *actions) { BOOL isDir = FALSE; BOOL isSymlink = FALSE; @@ -335,7 +339,9 @@ ChangeFileModeRecursivelyEnd: // 1. Recursive is only set on directories // 2. 'actions' is NULL if the mode is octal // -static BOOL ParseCommandLineArguments(__in int argc, __in wchar_t *argv[], +static BOOL ParseCommandLineArguments( + __in int argc, + __in_ecount(argc) wchar_t *argv[], __out BOOL *rec, __out_opt INT *mask, __out_opt PMODE_CHANGE_ACTION *actions, @@ -551,9 +557,9 @@ static INT ComputeNewMode(__in INT oldMode, // none // static BOOL ConvertActionsToMask(__in LPCWSTR path, - __in PMODE_CHANGE_ACTION actions, __out PINT puMask) + __in MODE_CHANGE_ACTION const *actions, __out PINT puMask) { - PMODE_CHANGE_ACTION curr = NULL; + MODE_CHANGE_ACTION const *curr = NULL; BY_HANDLE_FILE_INFORMATION fileInformation; DWORD dwErrorCode = ERROR_SUCCESS; @@ -608,7 +614,7 @@ static BOOL ConvertActionsToMask(__in LPCWSTR path, // none // static BOOL ChangeFileModeByActions(__in LPCWSTR path, - PMODE_CHANGE_ACTION actions) + MODE_CHANGE_ACTION const *actions) { INT mask = 0; @@ -769,10 +775,14 @@ static BOOL ParseMode(LPCWSTR modeString, PMODE_CHANGE_ACTION *pActions) switch (c) { case NULL: + __fallthrough; case L',': i++; + __fallthrough; case L'+': + __fallthrough; case L'-': + __fallthrough; case L'=': state = PARSE_MODE_ACTION_WHO_STATE; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/chown.c b/hadoop-common-project/hadoop-common/src/main/winutils/chown.c index 32ea77aa504..958699679a3 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/chown.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/chown.c @@ -116,13 +116,13 @@ ChangeFileOwnerByNameEnd: // Notes: // // -int Chown(int argc, wchar_t *argv[]) +int Chown(__in int argc, __in_ecount(argc) wchar_t *argv[]) { LPWSTR pathName = NULL; LPWSTR ownerInfo = NULL; - LPWSTR colonPos = NULL; + WCHAR const * colonPos = NULL; LPWSTR userName = NULL; size_t userNameLen = 0; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/groups.c b/hadoop-common-project/hadoop-common/src/main/winutils/groups.c index 1608c40ce75..9667d25326b 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/groups.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/groups.c @@ -32,12 +32,12 @@ // otherwise, space. // static BOOL PrintGroups( - LPLOCALGROUP_USERS_INFO_0 groups, + LOCALGROUP_USERS_INFO_0 const *groups, DWORD entries, BOOL formatOutput) { BOOL ret = TRUE; - LPLOCALGROUP_USERS_INFO_0 pTmpBuf = groups; + LOCALGROUP_USERS_INFO_0 const *pTmpBuf = groups; DWORD i; for (i = 0; i < entries; i++) @@ -80,7 +80,10 @@ static BOOL PrintGroups( // TRUE on the valid command line, FALSE otherwise // static BOOL ParseCommandLine( - int argc, wchar_t *argv[], wchar_t **user, BOOL *formatOutput) + __in int argc, + __in_ecount(argc) wchar_t *argv[], + __out PWSTR *user, + __out BOOL *formatOutput) { *formatOutput = FALSE; @@ -132,7 +135,7 @@ static BOOL ParseCommandLine( // Notes: // // -int Groups(int argc, wchar_t *argv[]) +int Groups(__in int argc, __in_ecount(argc) wchar_t *argv[]) { LPWSTR input = NULL; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/hardlink.c b/hadoop-common-project/hadoop-common/src/main/winutils/hardlink.c index 1be2541f041..76422a8772d 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/hardlink.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/hardlink.c @@ -37,7 +37,7 @@ typedef enum HardLinkCommandOptionType // TRUE: If the command line is valid // FALSE: otherwise static BOOL ParseCommandLine(__in int argc, - __in wchar_t *argv[], + __in_ecount(argc) wchar_t *argv[], __out HardLinkCommandOption *command) { *command = HardLinkInvalid; @@ -161,7 +161,7 @@ HardlinkCreateExit: // Returns: // EXIT_SUCCESS: On success // EXIT_FAILURE: otherwise -int Hardlink(int argc, wchar_t *argv[]) +int Hardlink(__in int argc, __in_ecount(argc) wchar_t *argv[]) { DWORD dwErrorCode = ERROR_SUCCESS; int ret = EXIT_FAILURE; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/include/winutils.h b/hadoop-common-project/hadoop-common/src/main/winutils/include/winutils.h index 34225fd8aa3..224c6544337 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/include/winutils.h +++ b/hadoop-common-project/hadoop-common/src/main/winutils/include/winutils.h @@ -20,13 +20,11 @@ #endif #pragma once - #include #include #include #include #include -#include #include #include @@ -85,25 +83,25 @@ enum WindowsAclMask extern const ACCESS_MASK WinMasks[]; -int Ls(int argc, wchar_t *argv[]); +int Ls(__in int argc, __in_ecount(argc) wchar_t *argv[]); void LsUsage(LPCWSTR program); -int Chmod(int argc, wchar_t *argv[]); +int Chmod(__in int argc, __in_ecount(argc) wchar_t *argv[]); void ChmodUsage(LPCWSTR program); -int Chown(int argc, wchar_t *argv[]); +int Chown(__in int argc, __in_ecount(argc) wchar_t *argv[]); void ChownUsage(LPCWSTR program); -int Groups(int argc, wchar_t *argv[]); +int Groups(__in int argc, __in_ecount(argc) wchar_t *argv[]); void GroupsUsage(LPCWSTR program); -int Hardlink(int argc, wchar_t *argv[]); +int Hardlink(__in int argc, __in_ecount(argc) wchar_t *argv[]); void HardlinkUsage(); -int Task(int argc, wchar_t *argv[]); +int Task(__in int argc, __in_ecount(argc) wchar_t *argv[]); void TaskUsage(); -int Symlink(int argc, wchar_t *argv[]); +int Symlink(__in int argc, __in_ecount(argc) wchar_t *argv[]); void SymlinkUsage(); int SystemInfo(); @@ -114,9 +112,9 @@ DWORD GetFileInformationByName(__in LPCWSTR pathName, __in BOOL followLink, DWORD ConvertToLongPath(__in PCWSTR path, __deref_out PWSTR *newPath); -DWORD GetSidFromAcctNameW(LPCWSTR acctName, PSID* ppSid); +DWORD GetSidFromAcctNameW(__in PCWSTR acctName, __out PSID* ppSid); -DWORD GetAccntNameFromSid(PSID pSid, LPWSTR *ppAcctName); +DWORD GetAccntNameFromSid(__in PSID pSid, __out LPWSTR *ppAcctName); void ReportErrorCode(LPCWSTR func, DWORD err); diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/libwinutils.c b/hadoop-common-project/hadoop-common/src/main/winutils/libwinutils.c index d21906638e8..61afd055cd1 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/libwinutils.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/libwinutils.c @@ -226,7 +226,6 @@ ConvertToLongPathExit: if (dwErrorCode != ERROR_SUCCESS) { LocalFree(newPathValue); - *newPath = NULL; } return dwErrorCode; @@ -398,7 +397,7 @@ DWORD JunctionPointCheck(__in LPCWSTR pathName, __out PBOOL res) // Notes: // Caller needs to destroy the memory of Sid by calling LocalFree() // -DWORD GetSidFromAcctNameW(LPCWSTR acctName, PSID *ppSid) +DWORD GetSidFromAcctNameW(__in PCWSTR acctName, __out PSID *ppSid) { DWORD dwSidSize = 0; DWORD cchDomainName = 0; @@ -545,7 +544,7 @@ static DWORD GetAccess(AUTHZ_CLIENT_CONTEXT_HANDLE hAuthzClient, { return GetLastError(); } - *pAccessRights = (*(PACCESS_MASK)(AccessReply.GrantedAccessMask)); + *pAccessRights = (*(const ACCESS_MASK *)(AccessReply.GrantedAccessMask)); return ERROR_SUCCESS; } @@ -1088,6 +1087,7 @@ DWORD ChangeFileModeByMask(__in LPCWSTR path, INT mode) DWORD revision = 0; PSECURITY_DESCRIPTOR pAbsSD = NULL; + PSECURITY_DESCRIPTOR pNonNullSD = NULL; PACL pAbsDacl = NULL; PACL pAbsSacl = NULL; PSID pAbsOwner = NULL; @@ -1200,7 +1200,8 @@ DWORD ChangeFileModeByMask(__in LPCWSTR path, INT mode) // present in the security descriptor, the DACL is replaced. The security // descriptor is then used to set the security of a file or directory. // - if (!SetSecurityDescriptorDacl(pAbsSD, TRUE, pNewDACL, FALSE)) + pNonNullSD = (pAbsSD != NULL) ? pAbsSD : pSD; + if (!SetSecurityDescriptorDacl(pNonNullSD, TRUE, pNewDACL, FALSE)) { ret = GetLastError(); goto ChangeFileModeByMaskEnd; @@ -1220,13 +1221,14 @@ DWORD ChangeFileModeByMask(__in LPCWSTR path, INT mode) // its parent, and the child objects will not lose their inherited permissions // from the current object. // - if (!SetFileSecurity(longPathName, DACL_SECURITY_INFORMATION, pAbsSD)) + if (!SetFileSecurity(longPathName, DACL_SECURITY_INFORMATION, pNonNullSD)) { ret = GetLastError(); goto ChangeFileModeByMaskEnd; } ChangeFileModeByMaskEnd: + pNonNullSD = NULL; LocalFree(longPathName); LocalFree(pSD); LocalFree(pNewDACL); @@ -1252,7 +1254,7 @@ ChangeFileModeByMaskEnd: // Notes: // Caller needs to destroy the memory of account name by calling LocalFree() // -DWORD GetAccntNameFromSid(PSID pSid, LPWSTR *ppAcctName) +DWORD GetAccntNameFromSid(__in PSID pSid, __out PWSTR *ppAcctName) { LPWSTR lpName = NULL; DWORD cchName = 0; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/ls.c b/hadoop-common-project/hadoop-common/src/main/winutils/ls.c index 8c9892d48a6..df94e3a59d5 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/ls.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/ls.c @@ -32,7 +32,7 @@ // altered. The caller need to initilize the mask string to be all '-' to get // the correct mask string. // -static BOOL GetMaskString(INT accessMask, LPWSTR maskString) +static BOOL GetMaskString(__in INT accessMask, __in_ecount(10) LPWSTR maskString) { if(wcslen(maskString) != 10) return FALSE; @@ -163,7 +163,10 @@ static wchar_t* CurrentDir = L"."; // TRUE on the valid command line, FALSE otherwise // BOOL ParseCommandLine( - int argc, wchar_t *argv[], wchar_t** path, int *optionsMask) + __in int argc, + __in_ecount(argc) wchar_t *argv[], + __deref_out PWSTR *path, + __out int *optionsMask) { int MaxOptions = 2; // Should be equal to the number of elems in CmdLineOption int i = 0; @@ -236,7 +239,7 @@ BOOL ParseCommandLine( // // Notes: // -int Ls(int argc, wchar_t *argv[]) +int Ls(__in int argc, __in_ecount(argc) wchar_t *argv[]) { LPWSTR pathName = NULL; LPWSTR longPathName = NULL; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/main.c b/hadoop-common-project/hadoop-common/src/main/winutils/main.c index 8e5f695ca80..94c374b407e 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/main.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/main.c @@ -19,7 +19,7 @@ static void Usage(LPCWSTR program); -int wmain(int argc, wchar_t* argv[]) +int wmain(__in int argc, __in_ecount(argc) wchar_t* argv[]) { LPCWSTR cmd = NULL; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c b/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c index 564459a4548..ff1779ce09f 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c @@ -28,7 +28,7 @@ // // Notes: // -int Symlink(int argc, wchar_t *argv[]) +int Symlink(__in int argc, __in_ecount(argc) wchar_t *argv[]) { PWSTR longLinkName = NULL; PWSTR longFileName = NULL; diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/systeminfo.c b/hadoop-common-project/hadoop-common/src/main/winutils/systeminfo.c index 00c0f0b6e16..7fce4241b95 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/systeminfo.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/systeminfo.c @@ -51,8 +51,8 @@ int SystemInfo() ULONGLONG cpuTimeMs; size_t size; LPBYTE pBuffer; - PPROCESSOR_POWER_INFORMATION ppi; - long cpuFrequencyKhz; + PROCESSOR_POWER_INFORMATION const *ppi; + ULONGLONG cpuFrequencyKhz; NTSTATUS status; ZeroMemory(&memInfo, sizeof(PERFORMANCE_INFORMATION)); @@ -98,11 +98,12 @@ int SystemInfo() LocalFree(pBuffer); return EXIT_FAILURE; } - ppi = (PPROCESSOR_POWER_INFORMATION)pBuffer; + ppi = (PROCESSOR_POWER_INFORMATION const *)pBuffer; cpuFrequencyKhz = ppi->MaxMhz*1000; LocalFree(pBuffer); - fwprintf_s(stdout, L"%Iu,%Iu,%Iu,%Iu,%Iu,%Iu,%Iu\n", vmemSize, memSize, vmemFree, memFree, sysInfo.dwNumberOfProcessors, cpuFrequencyKhz, cpuTimeMs); + fwprintf_s(stdout, L"%Iu,%Iu,%Iu,%Iu,%u,%I64u,%I64u\n", vmemSize, memSize, + vmemFree, memFree, sysInfo.dwNumberOfProcessors, cpuFrequencyKhz, cpuTimeMs); return EXIT_SUCCESS; } diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/task.c b/hadoop-common-project/hadoop-common/src/main/winutils/task.c index b8267cabaf6..19bda96a1e6 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/task.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/task.c @@ -50,7 +50,7 @@ typedef enum TaskCommandOptionType // TRUE: If the command line is valid // FALSE: otherwise static BOOL ParseCommandLine(__in int argc, - __in wchar_t *argv[], + __in_ecount(argc) wchar_t *argv[], __out TaskCommandOption *command) { *command = TaskInvalid; @@ -99,7 +99,7 @@ static BOOL ParseCommandLine(__in int argc, // Returns: // ERROR_SUCCESS: On success // GetLastError: otherwise -DWORD createTask(_TCHAR* jobObjName, _TCHAR* cmdLine) +DWORD createTask(__in PCWSTR jobObjName,__in PWSTR cmdLine) { DWORD err = ERROR_SUCCESS; DWORD exitCode = EXIT_FAILURE; @@ -138,7 +138,7 @@ DWORD createTask(_TCHAR* jobObjName, _TCHAR* cmdLine) // the child JVM uses this env var to send the task OS process identifier // to the TaskTracker. We pass the job object name. - if(SetEnvironmentVariable(_T("JVM_PID"), jobObjName) == 0) + if(SetEnvironmentVariable(L"JVM_PID", jobObjName) == 0) { err = GetLastError(); CloseHandle(jobObject); @@ -148,12 +148,14 @@ DWORD createTask(_TCHAR* jobObjName, _TCHAR* cmdLine) ZeroMemory( &si, sizeof(si) ); si.cb = sizeof(si); ZeroMemory( &pi, sizeof(pi) ); - if(CreateProcess(NULL, cmdLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi) == 0) + + if (CreateProcess(NULL, cmdLine, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi) == 0) { err = GetLastError(); CloseHandle(jobObject); return err; } + CloseHandle(pi.hThread); // Wait until child process exits. @@ -194,7 +196,7 @@ DWORD createTask(_TCHAR* jobObjName, _TCHAR* cmdLine) // Returns: // ERROR_SUCCESS: On success // GetLastError: otherwise -DWORD isTaskAlive(const _TCHAR* jobObjName, int* isAlive, int* procsInJob) +DWORD isTaskAlive(const WCHAR* jobObjName, int* isAlive, int* procsInJob) { PJOBOBJECT_BASIC_PROCESS_ID_LIST procList; HANDLE jobObject = NULL; @@ -254,7 +256,7 @@ DWORD isTaskAlive(const _TCHAR* jobObjName, int* isAlive, int* procsInJob) // Returns: // ERROR_SUCCESS: On success // GetLastError: otherwise -DWORD killTask(_TCHAR* jobObjName) +DWORD killTask(PCWSTR jobObjName) { HANDLE jobObject = OpenJobObject(JOB_OBJECT_TERMINATE, FALSE, jobObjName); if(jobObject == NULL) @@ -286,7 +288,7 @@ DWORD killTask(_TCHAR* jobObjName) // Returns: // ERROR_SUCCESS: On success // GetLastError: otherwise -DWORD printTaskProcessList(const _TCHAR* jobObjName) +DWORD printTaskProcessList(const WCHAR* jobObjName) { DWORD i; PJOBOBJECT_BASIC_PROCESS_ID_LIST procList; @@ -317,9 +319,9 @@ DWORD printTaskProcessList(const _TCHAR* jobObjName) numProcs = procList->NumberOfAssignedProcesses; LocalFree(procList); procList = (PJOBOBJECT_BASIC_PROCESS_ID_LIST) LocalAlloc(LPTR, sizeof (JOBOBJECT_BASIC_PROCESS_ID_LIST) + numProcs*32); - if (!procList) + if (procList == NULL) { - DWORD err = GetLastError(); + err = GetLastError(); CloseHandle(jobObject); return err; } @@ -343,7 +345,7 @@ DWORD printTaskProcessList(const _TCHAR* jobObjName) userTime.HighPart = user.dwHighDateTime; userTime.LowPart = user.dwLowDateTime; cpuTimeMs = (kernelTime.QuadPart+userTime.QuadPart)/10000; - _ftprintf_s(stdout, TEXT("%u,%Iu,%Iu,%Iu\n"), procList->ProcessIdList[i], pmc.PrivateUsage, pmc.WorkingSetSize, cpuTimeMs); + fwprintf_s(stdout, L"%Iu,%Iu,%Iu,%I64u\n", procList->ProcessIdList[i], pmc.PrivateUsage, pmc.WorkingSetSize, cpuTimeMs); } } CloseHandle( hProcess ); @@ -366,7 +368,7 @@ DWORD printTaskProcessList(const _TCHAR* jobObjName) // Returns: // ERROR_SUCCESS: On success // Error code otherwise: otherwise -int Task(int argc, wchar_t *argv[]) +int Task(__in int argc, __in_ecount(argc) wchar_t *argv[]) { DWORD dwErrorCode = ERROR_SUCCESS; TaskCommandOption command = TaskInvalid;