Merge pull request #19947 from hrydgard/code-cleanup

Fixes for minor code issues reported by PVS-Studio

Common/GPU/Vulkan/VulkanContext.cpp

@@ -858,14 +858,10 @@ VkResult VulkanContext::CreateDevice(int physical_device) {
 }
 
 VkResult VulkanContext::InitDebugUtilsCallback() {
+	VkDebugUtilsMessengerCreateInfoEXT callback1{VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT};
 	// We're intentionally skipping VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT and
 	// VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT, just too spammy.
-	int bits = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT
-		| VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT
-		| VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT;
-
-	VkDebugUtilsMessengerCreateInfoEXT callback1{VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT};
-	callback1.messageSeverity = bits;
+	callback1.messageSeverity = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT;
 	callback1.messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT;
 	callback1.pfnUserCallback = &VulkanDebugUtilsCallback;
 	callback1.pUserData = (void *)&g_LogOptions;

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -880,7 +880,7 @@ void VulkanRenderManager::EndCurRenderStep() {
 		if (!pipeline->pipeline[(size_t)rpType]) {
 			pipeline->pipeline[(size_t)rpType] = Promise<VkPipeline>::CreateEmpty();
 			_assert_(renderPass);
-			compileQueue_.push_back(CompileQueueEntry(pipeline, renderPass->Get(vulkan_, rpType, sampleCount), rpType, sampleCount));
+			compileQueue_.emplace_back(pipeline, renderPass->Get(vulkan_, rpType, sampleCount), rpType, sampleCount);
 			needsCompile = true;
 		}
 	}

Common/Math/math_util.h

@@ -9,18 +9,10 @@
 
 typedef unsigned short float16;
 
-// This ain't a 1.5.10 float16, it's a stupid hack format where we chop 16 bits off a float.
-// This choice is subject to change. Don't think I'm using this for anything at all now anyway.
-// DEPRECATED
-inline float16 FloatToFloat16(float x) {
-	int ix;
-	memcpy(&ix, &x, sizeof(float));
-	return ix >> 16;
-}
-
 inline float Float16ToFloat(float16 ix) {
+	uint32_t extended = ix;
 	float x;
-	memcpy(&x, &ix, sizeof(float));
+	memcpy(&x, &extended, sizeof(float));
 	return x;
 }
 

Common/Render/Text/draw_text.cpp

@@ -228,7 +228,7 @@ void TextDrawer::OncePerFrame() {
 			if (frameCount_ - iter->second->lastUsedFrame > 100) {
 				if (iter->second->texture)
 					iter->second->texture->Release();
-				cache_.erase(iter++);
+				iter = cache_.erase(iter);
 			} else {
 				iter++;
 			}
@@ -236,7 +236,7 @@ void TextDrawer::OncePerFrame() {
 
 		for (auto iter = sizeCache_.begin(); iter != sizeCache_.end(); ) {
 			if (frameCount_ - iter->second->lastUsedFrame > 100) {
-				sizeCache_.erase(iter++);
+				iter = sizeCache_.erase(iter);
 			} else {
 				iter++;
 			}

Common/UI/IconCache.cpp

@@ -189,7 +189,7 @@ void IconCache::Decimate(int64_t maxSize) {
 			if (iter->second.texture) {
 				iter->second.texture->Release();
 			}
-			cache_.erase(iter);
+			cache_.erase(iter);  // iter is recomputed above, so no need to set it.
 		}
 		sortEntries.pop_back();
 	}

Core/Core.cpp

@@ -566,7 +566,7 @@ void Core_MemoryExceptionInfo(u32 address, u32 accessSize, u32 pc, MemoryExcepti
 // Can't be ignored
 void Core_ExecException(u32 address, u32 pc, ExecExceptionType type) {
 	const char *desc = ExecExceptionTypeAsString(type);
-	WARN_LOG(Log::MemMap, "%s: Invalid exec address %08x PC %08x LR %08x", desc, address, pc, currentMIPS->r[MIPS_REG_RA]);
+	WARN_LOG(Log::MemMap, "%s: Invalid exec address %08x pc=%08x ra=%08x", desc, address, pc, currentMIPS->r[MIPS_REG_RA]);
 
 	MIPSExceptionInfo &e = g_exceptionInfo;
 	e = {};

Core/HLE/sceNetAdhoc.cpp

@@ -1089,10 +1089,11 @@ int WaitBlockingAdhocSocket(u64 threadSocketId, int type, int pspSocketId, void*
 
 void netAdhocValidateLoopMemory() {
 	// Allocate Memory if it wasn't valid/allocated after loaded from old SaveState
-	if (!dummyThreadHackAddr || (dummyThreadHackAddr && strcmp("dummythreadhack", kernelMemory.GetBlockTag(dummyThreadHackAddr)) != 0)) {
+	if (!dummyThreadHackAddr || strcmp("dummythreadhack", kernelMemory.GetBlockTag(dummyThreadHackAddr)) != 0) {
 		u32 blockSize = sizeof(dummyThreadCode);
 		dummyThreadHackAddr = kernelMemory.Alloc(blockSize, false, "dummythreadhack");
-		if (dummyThreadHackAddr) Memory::Memcpy(dummyThreadHackAddr, dummyThreadCode, sizeof(dummyThreadCode));
+		if (dummyThreadHackAddr)
+			Memory::Memcpy(dummyThreadHackAddr, dummyThreadCode, sizeof(dummyThreadCode));
 	}
 }
 
@@ -4919,6 +4920,10 @@ static int sceNetAdhocctlGetAddrByName(const char *nickName, u32 sizeAddr, u32 b
 	s32_le *buflen = NULL; //int32_t
 	if (Memory::IsValidAddress(sizeAddr)) buflen = (s32_le *)Memory::GetPointer(sizeAddr);
 
+	if (!nickName || !buflen) {
+		return hleLogError(Log::sceNet, ERROR_NET_ADHOCCTL_INVALID_ARG);
+	}
+
 	char nckName[ADHOCCTL_NICKNAME_LEN];
 	memcpy(nckName, nickName, ADHOCCTL_NICKNAME_LEN); // Copied to null-terminated var to prevent unexpected behaviour on Logs
 	nckName[ADHOCCTL_NICKNAME_LEN - 1] = 0;
@@ -4928,8 +4933,6 @@ static int sceNetAdhocctlGetAddrByName(const char *nickName, u32 sizeAddr, u32 b
 	// Library initialized
 	if (netAdhocctlInited)
 	{
-		// Valid Arguments
-		if (nickName != NULL && buflen != NULL)
 		{
 			SceNetAdhocctlPeerInfoEmu *buf = NULL;
 			if (Memory::IsValidAddress(bufAddr)) buf = (SceNetAdhocctlPeerInfoEmu *)Memory::GetPointer(bufAddr);

Core/HLE/sceUsbMic.cpp

@@ -59,36 +59,42 @@ static int micState; // 0 means stopped, 1 means started, for save state.
 static void __MicBlockingResume(u64 userdata, int cyclesLate) {
 	SceUID threadID = (SceUID)userdata;
 	u32 error;
-	int count = 0;
-	for (auto waitingThread : waitingThreads) {
-		if (waitingThread.threadID == threadID) {
-			SceUID waitID = __KernelGetWaitID(threadID, WAITTYPE_MICINPUT, error);
-			if (waitID == 0)
-				continue;
-			if (Microphone::isHaveDevice()) {
-				if (Microphone::getReadMicDataLength() >= waitingThread.needSize) {
-					u32 ret = __KernelGetWaitValue(threadID, error);
-					DEBUG_LOG(Log::HLE, "sceUsbMic: Waking up thread(%d)", (int)waitingThread.threadID);
-					__KernelResumeThreadFromWait(threadID, ret);
-					waitingThreads.erase(waitingThreads.begin() + count);
-				} else {
-					u64 waitTimeus = (waitingThread.needSize - Microphone::getReadMicDataLength()) * 1000000 / 2 / waitingThread.sampleRate;
-					CoreTiming::ScheduleEvent(usToCycles(waitTimeus), eventMicBlockingResume, userdata);
-				}
+	// On each path, we must either erase-iter-idiom, or increment iter
+	for (auto iter = waitingThreads.begin(); iter != waitingThreads.end();) {
+		if (iter->threadID != threadID) {
+			iter++;
+			continue;
+		}
+
+		SceUID waitID = __KernelGetWaitID(threadID, WAITTYPE_MICINPUT, error);
+		if (waitID == 0) {
+			iter++;
+			continue;
+		}
+
+		if (Microphone::isHaveDevice()) {
+			if (Microphone::getReadMicDataLength() >= iter->needSize) {
+				u32 ret = __KernelGetWaitValue(threadID, error);
+				DEBUG_LOG(Log::HLE, "sceUsbMic: Waking up thread(%d)", (int)iter->threadID);
+				__KernelResumeThreadFromWait(threadID, ret);
+				iter = waitingThreads.erase(iter);
 			} else {
-				for (int i = 0; i < waitingThread.needSize; i++) {
-					if (Memory::IsValidAddress(waitingThread.addr + i)) {
-						Memory::Write_U8(i & 0xFF, waitingThread.addr + i);
-					}
+				u64 waitTimeus = (iter->needSize - Microphone::getReadMicDataLength()) * 1000000 / 2 / iter->sampleRate;
+				CoreTiming::ScheduleEvent(usToCycles(waitTimeus), eventMicBlockingResume, userdata);
+				iter++;
+			}
+		} else {
+			for (int i = 0; i < iter->needSize; i++) {
+				if (Memory::IsValidAddress(iter->addr + i)) {
+					Memory::Write_U8(i & 0xFF, iter->addr + i);
 				}
-				u32 ret = __KernelGetWaitValue(threadID, error);
-				DEBUG_LOG(Log::HLE, "sceUsbMic: Waking up thread(%d)", (int)waitingThread.threadID);
-				__KernelResumeThreadFromWait(threadID, ret);
-				waitingThreads.erase(waitingThreads.begin() + count);
-				readMicDataLength += waitingThread.needSize;
 			}
+			u32 ret = __KernelGetWaitValue(threadID, error);
+			DEBUG_LOG(Log::HLE, "sceUsbMic: Waking up thread(%d)", (int)iter->threadID);
+			__KernelResumeThreadFromWait(threadID, ret);
+			readMicDataLength += iter->needSize;
+			iter = waitingThreads.erase(iter);
 		}
-		++count;
 	}
 }
 

Core/MIPS/MIPSVFPUUtils.h

@@ -232,7 +232,7 @@ static inline int GetNumVectorElements(VectorSize sz) {
 	case V_Pair:   return 2;
 	case V_Triple: return 3;
 	case V_Quad:   return 4;
-	default:       return 0;
+	default:       return 1;   // let's not confuse static analyzers more than necessary. Anyway, in reality this is unreachable.
 	}
 }
 

Core/MIPS/x86/CompVFPU.cpp

@@ -309,7 +309,7 @@ void Jit::Comp_SVQ(MIPSOpcode op) {
 			if (!g_Config.bFastMemory) {
 				DISABLE;
 			}
-			DISABLE;
+			DISABLE;  // The code below isn't quite working, so we fall back to interpreter for now.
 
 			gpr.MapReg(rs, true, false);
 			gpr.FlushLockX(ECX);

Core/MIPS/x86/RegCache.cpp

@@ -349,7 +349,7 @@ void GPRRegCache::MapReg(MIPSGPReg i, bool doLoad, bool makeDirty) {
 	if (!regs[i].away && regs[i].location.IsImm()) {
 		_assert_msg_(false, "Bad immediate");
 	}
-	if (!regs[i].away || (regs[i].away && regs[i].location.IsImm())) {
+	if (!regs[i].away || regs[i].location.IsImm()) {
 		X64Reg xr = GetFreeXReg();
 		_assert_msg_(!xregs[xr].dirty, "Xreg already dirty");
 		_assert_msg_(!xregs[xr].allocLocked, "GetFreeXReg returned locked register");

Core/Util/BlockAllocator.cpp

@@ -505,7 +505,6 @@ void BlockAllocator::Block::DoState(PointerWrap &p)
 	// Since we use truncate_cpy, the empty space is not zeroed.  Zero it now.
 	// This avoids saving uninitialized memory.
 	size_t tagLen = strlen(tag);
-	if (tagLen != sizeof(tag))
-		memset(tag + tagLen, 0, sizeof(tag) - tagLen);
+	memset(tag + tagLen, 0, sizeof(tag) - tagLen);
 	DoArray(p, tag, sizeof(tag));
 }

GPU/Common/TextureCacheCommon.cpp

@@ -822,7 +822,7 @@ void TextureCacheCommon::Decimate(TexCacheEntry *exceptThisOne, bool forcePressu
 			if (lowMemoryMode_ || iter->second->lastFrame + TEXTURE_SECOND_KILL_AGE < gpuStats.numFlips) {
 				ReleaseTexture(iter->second.get(), true);
 				secondCacheSizeEstimate_ -= EstimateTexMemoryUsage(iter->second.get());
-				secondCache_.erase(iter++);
+				iter = secondCache_.erase(iter);
 			} else {
 				++iter;
 			}
@@ -838,7 +838,7 @@ void TextureCacheCommon::Decimate(TexCacheEntry *exceptThisOne, bool forcePressu
 void TextureCacheCommon::DecimateVideos() {
 	for (auto iter = videos_.begin(); iter != videos_.end(); ) {
 		if (iter->flips + VIDEO_DECIMATE_AGE < gpuStats.numFlips) {
-			iter = videos_.erase(iter++);
+			iter = videos_.erase(iter);
 		} else {
 			++iter;
 		}

GPU/Software/SoftGpu.cpp

@@ -16,6 +16,7 @@
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
 #include <set>
+
 #include "Common/System/Display.h"
 #include "Common/GPU/OpenGL/GLFeatures.h"
 
@@ -430,9 +431,6 @@ SoftGPU::SoftGPU(GraphicsContext *gfxCtx, Draw::DrawContext *draw)
 	Rasterizer::Init();
 	Sampler::Init();
 	drawEngine_ = new SoftwareDrawEngine();
-	if (!drawEngine_)
-		return;
-
 	drawEngine_->SetGPUCommon(this);
 	drawEngine_->Init();
 	drawEngineCommon_ = drawEngine_;

UI/MainScreen.cpp

@@ -156,7 +156,6 @@ public:
 	}
 
 	bool Key(const KeyInput &key) override {
-		std::vector<int> pspKeys;
 		bool showInfo = false;
 
 		if (HasFocus() && UI::IsInfoKey(key)) {