From 01bab59e216b2c8d364ec713f4d93c75ec5243ff Mon Sep 17 00:00:00 2001 From: Dmitry Chapyshev Date: Sat, 30 Jul 2016 16:00:10 +0000 Subject: [PATCH] [WHOAMI] [ARP] [TRACERT] - Incorrect to compare the variable of BOOL type with TRUE. Any non-zero value is considered to be "true". [FREELDR] - Variable is assigned values twice - The 'strlen' function was called multiple times inside the body of a loop - It is inefficient to identify an empty string by using 'strlen(str) > 0' construct. A more efficient way is to check: str[0] != 0 [NTOBJSHEX] [SLAYER] [CMICONTROL] - It is inefficient to identify an empty string by using 'strlen(str) > 0' construct. A more efficient way is to check: str[0] != 0 [SHELL32] - There is no sense in testing the pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error - Verifying that a pointer value is not NULL is not required. The 'if (ptr != NULL)' check can be removed - Fix copy-paste error in CMenuFocusManager::PlaceHooks() [SRCLIENT] - Remove unneeded check. A part of conditional expression is always false. [DISK] [ATAPI] - Variable is assigned values twice * All bugs found by PVS-Studio svn path=/trunk/; revision=72059 --- reactos/base/applications/cmdutils/whoami/whoami.c | 4 ++-- reactos/base/applications/network/arp/arp.c | 2 +- reactos/base/applications/network/tracert/tracert.c | 4 ++-- reactos/boot/freeldr/freeldr/disk/scsiport.c | 1 - reactos/boot/freeldr/freeldr/lib/fs/fs.c | 10 ++++++++-- reactos/boot/freeldr/freeldr/ntldr/winldr.c | 2 +- reactos/boot/freeldr/freeldr/ntldr/wlregistry.c | 2 +- reactos/dll/shellext/ntobjshex/regfolder.cpp | 2 +- reactos/dll/shellext/slayer/slayer.c | 2 +- .../dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp | 3 +-- reactos/dll/win32/shell32/shellmenu/CMenuBand.cpp | 7 ++----- .../dll/win32/shell32/shellmenu/CMenuFocusManager.cpp | 2 +- reactos/dll/win32/srclient/srclient_main.c | 2 +- reactos/drivers/storage/class/disk/disk.c | 1 - reactos/drivers/storage/ide/atapi/atapi.c | 2 -- .../wdm/audio/drivers/CMIDriver/cmicontrol/main.cpp | 10 ++++------ 16 files changed, 26 insertions(+), 30 deletions(-) diff --git a/reactos/base/applications/cmdutils/whoami/whoami.c b/reactos/base/applications/cmdutils/whoami/whoami.c index 7efaa2787d1..485fb084b6d 100644 --- a/reactos/base/applications/cmdutils/whoami/whoami.c +++ b/reactos/base/applications/cmdutils/whoami/whoami.c @@ -753,7 +753,7 @@ int wmain(int argc, WCHAR* argv[]) { NoHeaderArgCount++; - if (NoHeader != TRUE) + if (NoHeader == FALSE) { NoHeader = TRUE; // wprintf(L"Headers disabled!\n"); @@ -788,7 +788,7 @@ int wmain(int argc, WCHAR* argv[]) /* looks like you can't use the "/fo list /nh" options together for some stupid reason */ - if (PrintFormat == list && NoHeader == TRUE) + if (PrintFormat == list && NoHeader != FALSE) { wprintf(WhoamiLoadRcString(IDS_ERROR_NH_LIST)); return 1; diff --git a/reactos/base/applications/network/arp/arp.c b/reactos/base/applications/network/arp/arp.c index 55576fce946..588b1a73cfc 100644 --- a/reactos/base/applications/network/arp/arp.c +++ b/reactos/base/applications/network/arp/arp.c @@ -464,7 +464,7 @@ INT Deletehost(PTCHAR pszInetAddr, PTCHAR pszIfAddr) pDelHost->dwIndex = pIpNetTable->table[0].dwIndex; } - if (bFlushTable == TRUE) + if (bFlushTable != FALSE) { /* delete arp cache */ if (FlushIpNetTable(pDelHost->dwIndex) != NO_ERROR) diff --git a/reactos/base/applications/network/tracert/tracert.c b/reactos/base/applications/network/tracert/tracert.c index 11f3ec14daa..d267d6cc087 100644 --- a/reactos/base/applications/network/tracert/tracert.c +++ b/reactos/base/applications/network/tracert/tracert.c @@ -450,7 +450,7 @@ Driver(PAPPINFO pInfo) /* run until we hit either max hops, or find the target */ while ((iHopCount <= pInfo->iMaxHops) && - (bFoundTarget != TRUE)) + (bFoundTarget == FALSE)) { USHORT iSeqNum = 0; INT i; @@ -460,7 +460,7 @@ Driver(PAPPINFO pInfo) /* run 3 pings for each hop */ for (i = 0; i < 3; i++) { - if (SetTTL(pInfo->icmpSock, iTTL) != TRUE) + if (SetTTL(pInfo->icmpSock, iTTL) == FALSE) { DebugPrint(_T("error in Setup()\n")); return ret; diff --git a/reactos/boot/freeldr/freeldr/disk/scsiport.c b/reactos/boot/freeldr/freeldr/disk/scsiport.c index 585d1d9713e..22cf72e8c33 100644 --- a/reactos/boot/freeldr/freeldr/disk/scsiport.c +++ b/reactos/boot/freeldr/freeldr/disk/scsiport.c @@ -607,7 +607,6 @@ ScsiPortGetPhysicalAddress( else { /* Nothing */ - *Length = 0; PhysicalAddress.QuadPart = (LONGLONG)(SP_UNINITIALIZED_VALUE); } diff --git a/reactos/boot/freeldr/freeldr/lib/fs/fs.c b/reactos/boot/freeldr/freeldr/lib/fs/fs.c index 77d14a07e7c..ff801d0e904 100644 --- a/reactos/boot/freeldr/freeldr/lib/fs/fs.c +++ b/reactos/boot/freeldr/freeldr/lib/fs/fs.c @@ -385,9 +385,12 @@ VOID FsSetFilePointer(PFILE FileHandle, ULONG NewFilePointer) ULONG FsGetNumPathParts(PCSTR Path) { size_t i; + size_t len; ULONG num; + + len = strlen(Path); - for (i = 0, num = 0; i < strlen(Path); i++) + for (i = 0, num = 0; i < len; i++) { if ((Path[i] == '\\') || (Path[i] == '/')) { @@ -410,11 +413,14 @@ ULONG FsGetNumPathParts(PCSTR Path) VOID FsGetFirstNameFromPath(PCHAR Buffer, PCSTR Path) { size_t i; + size_t len; + + len = strlen(Path); // Copy all the characters up to the end of the // string or until we hit a '\' character // and put them in Buffer - for (i = 0; i < strlen(Path); i++) + for (i = 0; i < len; i++) { if ((Path[i] == '\\') || (Path[i] == '/')) { diff --git a/reactos/boot/freeldr/freeldr/ntldr/winldr.c b/reactos/boot/freeldr/freeldr/ntldr/winldr.c index 51cb1367df6..de5135a15dd 100644 --- a/reactos/boot/freeldr/freeldr/ntldr/winldr.c +++ b/reactos/boot/freeldr/freeldr/ntldr/winldr.c @@ -650,7 +650,7 @@ LoadAndBootWindows(IN OperatingSystemItem* OperatingSystem, } /* Append a backslash if needed */ - if ((strlen(BootPath) == 0) || BootPath[strlen(BootPath) - 1] != '\\') + if ((BootPath[0] == 0) || BootPath[strlen(BootPath) - 1] != '\\') strcat(BootPath, "\\"); /* Read booting options */ diff --git a/reactos/boot/freeldr/freeldr/ntldr/wlregistry.c b/reactos/boot/freeldr/freeldr/ntldr/wlregistry.c index 3cf4e6ea2e4..888d2b123c1 100644 --- a/reactos/boot/freeldr/freeldr/ntldr/wlregistry.c +++ b/reactos/boot/freeldr/freeldr/ntldr/wlregistry.c @@ -705,7 +705,7 @@ WinLdrAddDriverToList(LIST_ENTRY *BootDriverListHead, // Check - if we have a valid ImagePath, if not - we need to build it // like "System32\\Drivers\\blah.sys" - if (ImagePath && (wcslen(ImagePath) > 0)) + if (ImagePath && (ImagePath[0] != 0)) { // Just copy ImagePath to the corresponding field in the structure PathLength = (USHORT)wcslen(ImagePath) * sizeof(WCHAR) + sizeof(UNICODE_NULL); diff --git a/reactos/dll/shellext/ntobjshex/regfolder.cpp b/reactos/dll/shellext/ntobjshex/regfolder.cpp index 4129f8a1bc1..bce65feadc3 100644 --- a/reactos/dll/shellext/ntobjshex/regfolder.cpp +++ b/reactos/dll/shellext/ntobjshex/regfolder.cpp @@ -471,7 +471,7 @@ HRESULT STDMETHODCALLTYPE CRegistryFolder::EnumObjects( SHCONTF grfFlags, IEnumIDList **ppenumIDList) { - if (wcslen(m_NtPath) == 0 && m_hRoot == NULL) + if (m_NtPath[0] == 0 && m_hRoot == NULL) { return GetEnumRegistryRoot(ppenumIDList); } diff --git a/reactos/dll/shellext/slayer/slayer.c b/reactos/dll/shellext/slayer/slayer.c index cf883a72f26..2c642d08e90 100644 --- a/reactos/dll/shellext/slayer/slayer.c +++ b/reactos/dll/shellext/slayer/slayer.c @@ -161,7 +161,7 @@ LoadAndParseAppCompatibilityFlags(LPCOMPATIBILITYPAGE info, } info->CSelectedItem = NULL; - if (_tcslen(szStr) > 0) + if (szStr[0] != 0) { PCITEM item; diff --git a/reactos/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp b/reactos/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp index 77d1095290a..e521b875a29 100644 --- a/reactos/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp +++ b/reactos/dll/win32/shell32/shelldesktop/CDesktopBrowser.cpp @@ -172,8 +172,7 @@ static CDesktopBrowser *SHDESK_Create(HWND hWnd, LPCREATESTRUCT lpCreateStruct) } pThis = new CComObject; - if (pThis == NULL) - return NULL; + pThis->AddRef(); hRet = pThis->Initialize(hWnd, ShellDesk); diff --git a/reactos/dll/win32/shell32/shellmenu/CMenuBand.cpp b/reactos/dll/win32/shell32/shellmenu/CMenuBand.cpp index 323e9a60391..56d0edfa99b 100644 --- a/reactos/dll/win32/shell32/shellmenu/CMenuBand.cpp +++ b/reactos/dll/win32/shell32/shellmenu/CMenuBand.cpp @@ -67,11 +67,8 @@ CMenuBand::~CMenuBand() { CMenuFocusManager::ReleaseManager(m_focusManager); - if (m_staticToolbar) - delete m_staticToolbar; - - if (m_SFToolbar) - delete m_SFToolbar; + delete m_staticToolbar; + delete m_SFToolbar; if (m_hmenu) DestroyMenu(m_hmenu); diff --git a/reactos/dll/win32/shell32/shellmenu/CMenuFocusManager.cpp b/reactos/dll/win32/shell32/shellmenu/CMenuFocusManager.cpp index 4301f90f2be..d8443e47c1e 100644 --- a/reactos/dll/win32/shell32/shellmenu/CMenuFocusManager.cpp +++ b/reactos/dll/win32/shell32/shellmenu/CMenuFocusManager.cpp @@ -680,7 +680,7 @@ LRESULT CMenuFocusManager::GetMsgHook(INT nCode, WPARAM hookWParam, LPARAM hookL HRESULT CMenuFocusManager::PlaceHooks() { - if (m_hMsgFilterHook) + if (m_hGetMsgHook) { WARN("GETMESSAGE hook already placed!\n"); return S_OK; diff --git a/reactos/dll/win32/srclient/srclient_main.c b/reactos/dll/win32/srclient/srclient_main.c index e3414e48a4a..68042c9cc21 100644 --- a/reactos/dll/win32/srclient/srclient_main.c +++ b/reactos/dll/win32/srclient/srclient_main.c @@ -35,7 +35,7 @@ SRSetRestorePointA(PRESTOREPOINTINFOA pRestorePtSpec, PSTATEMGRSTATUS pStateMgrS DPRINT1("SRSetRestorePointA is unimplemented\n"); - if (!pRestorePtSpec || !pRestorePtSpec->szDescription) + if (!pRestorePtSpec) return FALSE; RPInfoW.dwEventType = pRestorePtSpec->dwEventType; diff --git a/reactos/drivers/storage/class/disk/disk.c b/reactos/drivers/storage/class/disk/disk.c index 48c904ac8a1..f042031c467 100644 --- a/reactos/drivers/storage/class/disk/disk.c +++ b/reactos/drivers/storage/class/disk/disk.c @@ -505,7 +505,6 @@ Return Value: // SRB zone elements to allocate. // - adapterDisk = 0; adapterInfo = (PVOID) buffer; adapterDisk = ScsiClassFindUnclaimedDevices(InitializationData, adapterInfo); diff --git a/reactos/drivers/storage/ide/atapi/atapi.c b/reactos/drivers/storage/ide/atapi/atapi.c index 7988c85118f..a0b80525bf8 100644 --- a/reactos/drivers/storage/ide/atapi/atapi.c +++ b/reactos/drivers/storage/ide/atapi/atapi.c @@ -5483,7 +5483,6 @@ Return Value: // deviceExtension->ExpectingInterrupt = FALSE; - status = SRB_STATUS_SUCCESS; } else { @@ -5495,7 +5494,6 @@ Return Value: GetBaseStatus(baseIoAddress1, statusByte); deviceExtension->ExpectingInterrupt = FALSE; - status = SRB_STATUS_SUCCESS; if (errorByte & IDE_ERROR_DATA_ERROR) { diff --git a/reactos/drivers/wdm/audio/drivers/CMIDriver/cmicontrol/main.cpp b/reactos/drivers/wdm/audio/drivers/CMIDriver/cmicontrol/main.cpp index 22b6a176ebe..6649e4d4841 100644 --- a/reactos/drivers/wdm/audio/drivers/CMIDriver/cmicontrol/main.cpp +++ b/reactos/drivers/wdm/audio/drivers/CMIDriver/cmicontrol/main.cpp @@ -943,12 +943,10 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PSTR szCmdLine, ZeroMemory(&cmiTopologyDev, sizeof(CMIDEV)); hWave = NULL; - if (szCmdLine) { - if (strlen(szCmdLine) > 0) { - int result = parseArguments(szCmdLine); - cleanUp(); - return result; - } + if (szCmdLine && szCmdLine[0] != 0) { + int result = parseArguments(szCmdLine); + cleanUp(); + return result; } if ((hWndMain = FindWindow("cmiControlPanel", NULL))) { -- 2.17.1