Merge pull request #19517 from hrydgard/more-cleanups

Fix minor code issues flagged by PVS-Studio and reported by alphrixus.

Common/Data/Format/JSONWriter.cpp

@@ -19,17 +19,17 @@ JsonWriter::~JsonWriter() {
 
 void JsonWriter::begin() {
 	str_ << "{";
-	stack_.push_back(StackEntry(DICT));
+	stack_.emplace_back(DICT);
 }
 
 void JsonWriter::beginArray() {
 	str_ << "[";
-	stack_.push_back(StackEntry(ARRAY));
+	stack_.emplace_back(ARRAY);
 }
 
 void JsonWriter::beginRaw() {
 	// For the uncommon case of writing a value directly, to avoid duplicated code.
-	stack_.push_back(StackEntry(RAW));
+	stack_.emplace_back(RAW);
 }
 
 void JsonWriter::end() {
@@ -84,7 +84,7 @@ const char *JsonWriter::arrayComma() const {
 void JsonWriter::pushDict() {
 	str_ << arrayComma() << arrayIndent() << "{";
 	stack_.back().first = false;
-	stack_.push_back(StackEntry(DICT));
+	stack_.emplace_back(DICT);
 }
 
 void JsonWriter::pushDict(const std::string &name) {
@@ -92,20 +92,20 @@ void JsonWriter::pushDict(const std::string &name) {
 	writeEscapedString(name);
 	str_ << (pretty_ ? "\": {" : "\":{");
 	stack_.back().first = false;
-	stack_.push_back(StackEntry(DICT));
+	stack_.emplace_back(DICT);
 }
 
 void JsonWriter::pushArray() {
 	str_ << arrayComma() << arrayIndent() << "[";
 	stack_.back().first = false;
-	stack_.push_back(StackEntry(ARRAY));
+	stack_.emplace_back(ARRAY);
 }
 
 void JsonWriter::pushArray(const std::string &name) {
 	str_ << comma() << indent() << "\"";
 	writeEscapedString(name);
 	str_ << (pretty_ ? "\": [" : "\":[");
-	stack_.push_back(StackEntry(ARRAY));
+	stack_.emplace_back(ARRAY);
 }
 
 void JsonWriter::writeBool(bool value) {

Common/File/PathBrowser.cpp

@@ -52,10 +52,11 @@ bool LoadRemoteFileList(const Path &url, const std::string &userAgent, bool *can
 	std::vector<std::string> items;
 	result.TakeAll(&listing);
 
+	constexpr std::string_view ContentTypeHeader = "Content-Type:";
 	std::string contentType;
 	for (const std::string &header : responseHeaders) {
-		if (startsWithNoCase(header, "Content-Type:")) {
-			contentType = header.substr(strlen("Content-Type:"));
+		if (startsWithNoCase(header, ContentTypeHeader)) {
+			contentType = header.substr(ContentTypeHeader.size());
 			// Strip any whitespace (TODO: maybe move this to stringutil?)
 			contentType.erase(0, contentType.find_first_not_of(" \t\r\n"));
 			contentType.erase(contentType.find_last_not_of(" \t\r\n") + 1);

Common/File/VFS/ZipFileReader.cpp

@@ -109,13 +109,14 @@ bool ZipFileReader::GetFileListing(const char *orig_path, std::vector<File::File
 
 	// INFO_LOG(Log::System, "Zip: Listing '%s'", orig_path);
 
+	const std::string relativePath = path.substr(inZipPath_.size());
+
 	listing->reserve(directories.size() + files.size());
 	for (const auto &dir : directories) {
 		File::FileInfo info;
 		info.name = dir;
 
 		// Remove the "inzip" part of the fullname.
-		std::string relativePath = std::string(path).substr(inZipPath_.size());
 		info.fullName = Path(relativePath + dir);
 		info.exists = true;
 		info.isWritable = false;
@@ -125,10 +126,8 @@ bool ZipFileReader::GetFileListing(const char *orig_path, std::vector<File::File
 	}
 
 	for (const auto &fiter : files) {
-		std::string fpath = path;
 		File::FileInfo info;
 		info.name = fiter;
-		std::string relativePath = std::string(path).substr(inZipPath_.size());
 		info.fullName = Path(relativePath + fiter);
 		info.exists = true;
 		info.isWritable = false;

Common/Net/Sinks.cpp

@@ -284,7 +284,6 @@ size_t OutputSink::PushAtMost(const char *buf, size_t bytes) {
 	return avail;
 }
 
-
 bool OutputSink::Printf(const char *fmt, ...) {
 	// Let's start by checking how much space we have.
 	size_t avail = BUFFER_SIZE - std::max(write_, valid_);
@@ -301,10 +300,10 @@ bool OutputSink::Printf(const char *fmt, ...) {
 	if (result >= (int)avail) {
 		// There wasn't enough space.  Let's use a buffer instead.
 		// This could be caused by wraparound.
-		char temp[BUFFER_SIZE];
-		result = vsnprintf(temp, BUFFER_SIZE, fmt, args);
+		char temp[4096];
+		result = vsnprintf(temp, sizeof(temp), fmt, args);
 
-		if ((size_t)result < BUFFER_SIZE && result > 0) {
+		if ((size_t)result < sizeof(temp) && result > 0) {
 			// In case it did return the null terminator.
 			if (temp[result - 1] == '\0') {
 				result--;

Core/ControlMapper.cpp

@@ -85,11 +85,6 @@ static int GetOppositeVKey(int vkey) {
 	}
 }
 
-static bool IsAxisVKey(int vkey) {
-	// Little hacky but works, of course.
-	return GetOppositeVKey(vkey) != 0;
-}
-
 static bool IsUnsignedMapping(int vkey) {
 	return vkey == VIRTKEY_SPEED_ANALOG;
 }

Core/Debugger/WebSocket/InputSubscriber.cpp

@@ -181,7 +181,7 @@ void WebSocketInputState::ButtonsPress(DebuggerRequest &req) {
 	press.duration = 1;
 	if (!req.ParamU32("duration", &press.duration, false, DebuggerParamType::OPTIONAL))
 		return;
-	if (press.duration < 0)
+	if ((int)press.duration < 0)
 		return req.Fail("Parameter 'duration' must not be negative");
 	const JsonNode *value = req.data.get("ticket");
 	press.ticket = value ? json_stringify(value) : "";

Core/HLE/sceUsbMic.cpp

@@ -429,8 +429,6 @@ u32 __MicInput(u32 maxSamples, u32 sampleRate, u32 bufAddr, MICTYPE type, bool b
 	} else {
 		audioBuf->resize(size);
 	}
-	if (!audioBuf)
-		return 0;
 
 	numNeedSamples = maxSamples;
 	readMicDataLength = 0;

Core/MIPS/IR/IRPassSimplify.cpp

@@ -1820,7 +1820,7 @@ bool ApplyMemoryValidation(const IRWriter &in, IRWriter &out, const IROptions &o
 	for (IRInst inst : in.GetInstructions()) {
 		IRMemoryOpInfo info = IROpMemoryAccessSize(inst.op);
 		// Note: we only combine word aligned accesses.
-		if (info.size != 0 && inst.src1 == MIPS_REG_SP && info.size == 4) {
+		if (info.size == 4 && inst.src1 == MIPS_REG_SP) {
 			if (spModified) {
 				// No good, it was modified and then we did more accesses.  Can't combine.
 				spUpper = -1;

Core/RetroAchievements.cpp

@@ -255,6 +255,8 @@ bool IsActive() {
 	return GetGameID() != 0;
 }
 
+#ifdef RC_CLIENT_SUPPORTS_RAINTEGRATION
+
 static void raintegration_write_memory_handler(uint32_t address, uint8_t *buffer, uint32_t num_bytes, rc_client_t *client) {
 	// convert_retroachievements_address_to_real_address
 	uint32_t realAddress = address + PSP_MEMORY_OFFSET;
@@ -264,6 +266,8 @@ static void raintegration_write_memory_handler(uint32_t address, uint8_t *buffer
 	}
 }
 
+#endif
+
 static uint32_t read_memory_callback(uint32_t address, uint8_t *buffer, uint32_t num_bytes, rc_client_t *client) {
 	// Achievements are traditionally defined relative to the base of main memory of the emulated console.
 	// This is some kind of RetroArch-related legacy. In the PSP's case, this is simply a straight offset of 0x08000000.

GPU/Common/FramebufferManagerCommon.cpp

@@ -3182,48 +3182,50 @@ bool FramebufferManagerCommon::ReadbackStencilbuffer(Draw::Framebuffer *fbo, int
 }
 
 void FramebufferManagerCommon::ReadFramebufferToMemory(VirtualFramebuffer *vfb, int x, int y, int w, int h, RasterChannel channel, Draw::ReadbackMode mode) {
+	if (!vfb || !vfb->fbo) {
+		return;
+	}
+
 	// Clamp to bufferWidth. Sometimes block transfers can cause this to hit.
 	if (x + w >= vfb->bufferWidth) {
 		w = vfb->bufferWidth - x;
 	}
-	if (vfb && vfb->fbo) {
-		if (gameUsesSequentialCopies_) {
-			// Ignore the x/y/etc., read the entire thing.  See below.
-			x = 0;
-			y = 0;
-			w = vfb->width;
-			h = vfb->height;
+	if (gameUsesSequentialCopies_) {
+		// Ignore the x/y/etc., read the entire thing.  See below.
+		x = 0;
+		y = 0;
+		w = vfb->width;
+		h = vfb->height;
+		vfb->memoryUpdated = true;
+		vfb->usageFlags |= FB_USAGE_DOWNLOAD;
+	} else if (x == 0 && y == 0 && w == vfb->width && h == vfb->height) {
+		// Mark it as fully downloaded until next render to it.
+		if (channel == RASTER_COLOR)
 			vfb->memoryUpdated = true;
-			vfb->usageFlags |= FB_USAGE_DOWNLOAD;
-		} else if (x == 0 && y == 0 && w == vfb->width && h == vfb->height) {
-			// Mark it as fully downloaded until next render to it.
-			if (channel == RASTER_COLOR)
-				vfb->memoryUpdated = true;
-			vfb->usageFlags |= FB_USAGE_DOWNLOAD;
-		} else {
-			// Let's try to set the flag eventually, if the game copies a lot.
-			// Some games (like Grand Knights History) copy subranges very frequently.
-			const static int FREQUENT_SEQUENTIAL_COPIES = 3;
-			static int frameLastCopy = 0;
-			static u32 bufferLastCopy = 0;
-			static int copiesThisFrame = 0;
-			if (frameLastCopy != gpuStats.numFlips || bufferLastCopy != vfb->fb_address) {
-				frameLastCopy = gpuStats.numFlips;
-				bufferLastCopy = vfb->fb_address;
-				copiesThisFrame = 0;
-			}
-			if (++copiesThisFrame > FREQUENT_SEQUENTIAL_COPIES) {
-				gameUsesSequentialCopies_ = true;
-			}
+		vfb->usageFlags |= FB_USAGE_DOWNLOAD;
+	} else {
+		// Let's try to set the flag eventually, if the game copies a lot.
+		// Some games (like Grand Knights History) copy subranges very frequently.
+		const static int FREQUENT_SEQUENTIAL_COPIES = 3;
+		static int frameLastCopy = 0;
+		static u32 bufferLastCopy = 0;
+		static int copiesThisFrame = 0;
+		if (frameLastCopy != gpuStats.numFlips || bufferLastCopy != vfb->fb_address) {
+			frameLastCopy = gpuStats.numFlips;
+			bufferLastCopy = vfb->fb_address;
+			copiesThisFrame = 0;
+		}
+		if (++copiesThisFrame > FREQUENT_SEQUENTIAL_COPIES) {
+			gameUsesSequentialCopies_ = true;
 		}
-
-		// This handles any required stretching internally.
-		ReadbackFramebuffer(vfb, x, y, w, h, channel, mode);
-
-		draw_->Invalidate(InvalidationFlags::CACHED_RENDER_STATE);
-		textureCache_->ForgetLastTexture();
-		RebindFramebuffer("RebindFramebuffer - ReadFramebufferToMemory");
 	}
+
+	// This handles any required stretching internally.
+	ReadbackFramebuffer(vfb, x, y, w, h, channel, mode);
+
+	draw_->Invalidate(InvalidationFlags::CACHED_RENDER_STATE);
+	textureCache_->ForgetLastTexture();
+	RebindFramebuffer("RebindFramebuffer - ReadFramebufferToMemory");
 }
 
 void FramebufferManagerCommon::FlushBeforeCopy() {

UI/ControlMappingScreen.cpp

@@ -730,8 +730,8 @@ public:
 		return this;
 	}
 
-	MockButton *SetFlipHBG(float f) {
-		flipHBG_ = f;
+	MockButton *SetFlipHBG(bool b) {
+		flipHBG_ = b;
 		return this;
 	}
 
@@ -750,7 +750,7 @@ public:
 		return selectedButton_ && *selectedButton_ == button_;
 	}
 
-	int Button() {
+	int Button() const {
 		return button_;
 	}
 
@@ -776,7 +776,7 @@ class MockPSP : public UI::AnchorLayout {
 public:
 	static constexpr float SCALE = 1.4f;
 
-	MockPSP(UI::LayoutParams *layoutParams = nullptr);
+	explicit MockPSP(UI::LayoutParams *layoutParams = nullptr);
 	void SelectButton(int btn);
 	void FocusButton(int btn);
 	void NotifyPressed(int btn);