Merge pull request #22225 from hrydgard/misc-fixes

Assortment of minor fixes

Core/CmdLine.cpp

@@ -195,6 +195,7 @@ static const CommandLineParam g_autoParams[] = {
 	{POFF(root), CmdParamType::String, "root", 'r', "Mount directory as the root of host0:/."},
 	{POFF(memStick), CmdParamType::String, "memstick", '\0', "Memory stick root directory (contains PSP/GAME etc)"},
 	{POFF(stateToLoad), CmdParamType::String, "state", '\0', "Load state from specified file"},
+	{POFF(stateToSave), CmdParamType::String, "save-state", '\0', "Save a state to FILE partway through the run", CmdLineMode::Headless},
 	{POFF(compare), CmdParamType::Bool, "compare", 'c', "Enable comparison mode", CmdLineMode::Headless},
 	{POFF(bench), CmdParamType::Bool, "bench", 'b', "Enable benchmark mode", CmdLineMode::Headless},
 	{POFF(oldAtrac), CmdParamType::Bool, "old-atrac", '\0', "Use old ATRAC decoder"},

Core/CmdLine.h

@@ -66,6 +66,7 @@ struct CommandLineOptions {
 	// meant copying it in. Points at the same layout the app uses, so the two can share one.
 	std::optional<std::string> memStick;
 	std::optional<std::string> stateToLoad;
+	std::optional<std::string> stateToSave;
 
 	// Headless: unpack the firmware inside an official updater EBOOT.PBP (given as the boot
 	// filename) into this directory, then exit without booting anything.

Core/HLE/sceAudio.cpp

@@ -185,9 +185,11 @@ static int sceAudioGetChannelRestLength(u32 chan) {
 	return hleLogVerbose(Log::sceAudio, remainingSamples);
 }
 
-static u32 GetFreeChannel() {
+static int GetFreeChannel() {
 	// Changed this to allow channel 0. Fixes the startup sound in VSH. TODO: Why did we not allow channel 0 before?
-	for (u32 i = PSP_AUDIO_CHANNEL_MAX - 1; i >= 0; --i) {
+	// The counter has to be signed: as a u32 the i >= 0 condition is always true, so with every
+	// channel reserved it wrapped past zero and ran off the array instead of giving up.
+	for (int i = PSP_AUDIO_CHANNEL_MAX - 1; i >= 0; --i) {
 		if (!g_audioChans[i].reserved)
 			return i;
 	}
@@ -373,7 +375,7 @@ static u32 sceAudioSetVolumeOffset() {
 	return 0;
 }
 
-static bool SRCFrequencyAllowed(int freq) {
+bool SRCFrequencyAllowed(int freq) {
 	if (freq == 44100 || freq == 22050 || freq == 11025)
 		return true;
 	if (freq == 48000 || freq == 32000 || freq == 24000 || freq == 16000 || freq == 12000 || freq == 8000)

Core/HLE/sceAudio.h

@@ -63,5 +63,8 @@ struct AudioChannel {
 // The extra channel is for SRC/Output2/Vaudio (who all share, apparently.)
 extern AudioChannel g_audioChans[PSP_AUDIO_CHANNEL_MAX + 1];
 
+// The sample rates the SRC and VAUDIO channels will accept.
+bool SRCFrequencyAllowed(int freq);
+
 void Register_sceAudio();
 

Core/HLE/sceIo.cpp

@@ -2096,12 +2096,32 @@ static u32 sceIoDevctl(const char *name, int cmd, u32 argAddr, int argLen, u32 o
 	return hleNoLog(SCE_KERNEL_ERROR_UNSUP);
 }
 
+static bool IoPathHasWildcard(const char *path) {
+	return path && strpbrk(path, "*?") != nullptr;
+}
+
 static u32 sceIoRename(const char *from, const char *to) {
 	// TODO: Timing isn't terribly accurate.
+
+	// sceIoRename doesn't expand wildcards, it refuses them - in either path, and before it
+	// looks at whether anything is actually there.
+	if (IoPathHasWildcard(from) || IoPathHasWildcard(to)) {
+		return hleDelayResult(hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "wildcard in path"), "file renamed", 1000);
+	}
+
 	if (!pspFileSystem.GetFileInfo(from).exists)
 		return hleDelayResult(hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_FILE_NOT_FOUND), "file renamed", 1000);
 
+	// The PSP won't rename onto something that already exists, and renaming a file onto itself
+	// counts. Host rename() would happily replace the destination.
+	if (pspFileSystem.GetFileInfo(to).exists)
+		return hleDelayResult(hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_FILE_ALREADY_EXISTS, "destination exists"), "file renamed", 1000);
+
 	int result = pspFileSystem.RenameFile(from, to);
+	if (result == (int)SCE_KERNEL_ERROR_XDEV) {
+		// Renaming across devices is refused up front, without the wait the other errors take.
+		return hleLogError(Log::sceIo, result, "cannot rename across devices");
+	}
 	if (result < 0)
 		WARN_LOG(Log::sceIo, "Could not move %s to %s", from, to);
 	return hleDelayResult(hleLogDebug(Log::sceIo, result), "file renamed", 1000);

Core/HLE/sceUtility.cpp

@@ -46,6 +46,7 @@
 #include "Core/HLE/sceAtrac.h"
 #include "Core/HLE/sceUtility.h"
 #include "Core/HLE/sceNet.h"
+#include "Core/HLE/sceNetAdhoc.h"
 
 #include "Core/Dialog/PSPDialog.h"
 #include "Core/Dialog/PSPSaveDialog.h"
@@ -1220,19 +1221,23 @@ static u32 sceUtilitySetSystemParamString(u32 id, u32 strPtr)
 }
 
 static u32 sceUtilityGetSystemParamString(u32 id, u32 destAddr, int destSize) {
-	if (!Memory::IsValidRange(destAddr, destSize)) {
+	// A size that isn't positive can't hold the string, and that's what the PSP reports - not a
+	// bad-buffer error. Range checking it first would turn a negative size into a huge range.
+	if (destSize > 0 && !Memory::IsValidRange(destAddr, destSize)) {
 		// TODO: What error code?
 		return hleLogError(Log::sceUtility, -1);
 	}
-	char *buf = (char *)Memory::GetPointerWriteUnchecked(destAddr);
 	switch (id) {
 	case PSP_SYSTEMPARAM_ID_STRING_NICKNAME:
+	{
 		// If there's not enough space for the string and null terminator, fail.
 		if (destSize <= (int)g_Config.sNickName.length())
 			return SCE_ERROR_UTILITY_STRING_TOO_LONG;
+		char *buf = (char *)Memory::GetPointerWriteUnchecked(destAddr);
 		// TODO: should we zero-pad the output as strncpy does? And what are the semantics for the terminating null if destSize == length?
 		strncpy(buf, g_Config.sNickName.c_str(), destSize);
 		break;
+	}
 
 	default:
 		return hleLogError(Log::sceUtility, SCE_ERROR_UTILITY_INVALID_SYSTEM_PARAM_ID);
@@ -1263,7 +1268,10 @@ static u32 sceUtilityGetSystemParamInt(u32 id, u32 destaddr) {
 	switch (id) {
 	case PSP_SYSTEMPARAM_ID_INT_ADHOC_CHANNEL:
 		param = g_Config.iWlanAdhocChannel;
-		if (param == PSP_SYSTEMPARAM_ADHOC_CHANNEL_AUTOMATIC) {
+		// Only once adhocctl is up. The FIXME below wondered whether this error depends on that,
+		// and it does - utility/systemparam gets a plain 0 out of the hardware before any adhoc
+		// module is initialized, which is the state nearly every game asks this in.
+		if (param == PSP_SYSTEMPARAM_ADHOC_CHANNEL_AUTOMATIC && netAdhocctlInited) {
 			// FIXME: Actually.. it's always returning 0x800ADF4 regardless using Auto channel or Not, and regardless the connection state either,
 			//        Not sure whether this error code only returned after Adhocctl Initialized (ie. netAdhocctlInited) or also before initialized.
 			// FIXME: Outputted channel (might be unchanged?) either 0 when not connected to a group yet (ie. adhocctlState == ADHOCCTL_STATE_DISCONNECTED),

Core/HLE/sceVaudio.cpp

@@ -42,6 +42,20 @@ void __VaudioDoState(PointerWrap &p) {
 }
 
 static u32 sceVaudioChReserve(int sampleCount, int freq, int format) {
+	// The vaudio channel is pickier than the normal ones: only three sample counts, stereo only,
+	// and the same set of sample rates the SRC channel takes.
+	if (format != 2) {
+		ERROR_LOG(Log::sceAudio, "sceVaudioChReserve(%i, %i, %i) - unexpected format", sampleCount, freq, format);
+		return SCE_KERNEL_ERROR_INVALID_FORMAT;
+	}
+	if (sampleCount != 256 && sampleCount != 1024 && sampleCount != 2048) {
+		ERROR_LOG(Log::sceAudio, "sceVaudioChReserve(%i, %i, %i) - invalid sample count", sampleCount, freq, format);
+		return SCE_KERNEL_ERROR_INVALID_SIZE;
+	}
+	if (freq != 0 && !SRCFrequencyAllowed(freq)) {
+		ERROR_LOG(Log::sceAudio, "sceVaudioChReserve(%i, %i, %i) - invalid frequency", sampleCount, freq, format);
+		return SCE_ERROR_AUDIO_INVALID_FREQUENCY;
+	}
 	if (vaudioReserved) {
 		ERROR_LOG(Log::sceAudio, "sceVaudioChReserve(%i, %i, %i) - already reserved", sampleCount, freq, format);
 		return SCE_KERNEL_ERROR_BUSY;

headless/Headless.cpp

@@ -84,6 +84,9 @@ static bool g_writeFailureScreenshot = true;
 static bool g_writeDebugOutput = true;
 // Set from the savestate callback on the emu thread, read after it has been joined.
 static bool g_stateLoadFailed = false;
+// Set by --save-state. Saving needs the game to actually be running, so it happens from the run
+// loop rather than up front like a load does.
+static std::string g_stateToSave;
 
 #if PPSSPP_PLATFORM(ANDROID)
 JNIEnv *getEnv() {
@@ -338,13 +341,24 @@ static bool RunAutoTest(GraphicsContext *graphicsContext, CoreParameter &corePar
 	}
 
 	bool passed = true;
-	double deadline = time_now_d() + opt.timeout;
+	const double startTime = time_now_d();
+	double deadline = startTime + opt.timeout;
+	// Late enough that the game is past booting, early enough to leave the run some time after.
+	double saveStateAt = startTime + opt.timeout * 0.7;
 	coreState = coreParameter.startBreak ? CORE_STEPPING_CPU : CORE_RUNNING_CPU;
 	while (coreState == CORE_RUNNING_CPU || coreState == CORE_STEPPING_CPU) {
 		// Savestate loads/saves are queued and applied here, same as EmuScreen::render does in the
 		// app. Without this, --state silently did nothing at all.
 		SaveState::Process();
 
+		if (!g_stateToSave.empty() && time_now_d() > saveStateAt) {
+			const std::string filename = g_stateToSave;
+			g_stateToSave.clear();
+			SaveState::Save(Path(filename), -1, [](SaveState::Status status, std::string_view message, std::string_view) {
+				fprintf(stderr, "%.*s\n", (int)message.size(), message.data());
+			});
+		}
+
 		int blockTicks = (int)usToCycles(1000000 / 10);
 		PSP_RunLoopFor(blockTicks);
 
@@ -878,6 +892,10 @@ int main(int argc, const char* argv[]) {
 		}
 	}
 
+	if (cmdLineOptions.stateToSave.has_value()) {
+		g_stateToSave = cmdLineOptions.stateToSave.value();
+	}
+
 	if (stateToLoad) {
 		// Queued now, actually applied by SaveState::Process() once the game is up and running.
 		SaveState::Load(Path(stateToLoad), -1, [](SaveState::Status status, std::string_view message, std::string_view) {

test.py

@@ -211,6 +211,7 @@ tests_good = [
   "intr/suspended",
   "intr/vblank/vblank",
   "io/cwd/cwd",
+  "io/file/rename",
   "io/open/badparent",
   "jpeg/create",
   "jpeg/delete",
@@ -362,6 +363,7 @@ tests_good = [
   "utility/savedata/autosave",
   "utility/savedata/filelist",
   "utility/savedata/makedata",
+  "utility/systemparam/systemparam",
   "umd/callbacks/umd",
   "umd/register",
   "video/mpeg/ringbuffer/avail",
@@ -451,7 +453,6 @@ tests_next = [
   "intr/waits",
   "io/directory/directory",
   "io/file/file",
-  "io/file/rename",
   "io/io/io",
   "io/iodrv/iodrv",
   "io/open/tty0",
@@ -496,7 +497,6 @@ tests_next = [
   #"utility/savedata/saveemptyfilename",
   "utility/savedata/secureversion",
   "utility/savedata/sizes",
-  "utility/systemparam/systemparam",
   "video/mpeg/basic",
   "video/pmf/pmf",
   "video/pmf_simple/pmf_simple",