libretro / mupen64plus-libretro-nx

Improved mupen64plus libretro core reimplementation
GNU General Public License v2.0
231 stars 115 forks source link

64DD titles can crash if stored on a read only volume as it tries to create ".disk_save" file. #460

Closed 7oxicshadow closed 2 years ago

7oxicshadow commented 2 years ago

As per the title, If a .disk_save file is required it will fail to create on a read only file system.

The following function blindly assumes that it can write to the path:

const char *get_dd_disk_save_path(const char* diskname, int save_format)
{
    char* newPath = (char*)malloc(PATH_MAX);
    strcpy(newPath, diskname);

    if(save_format == 0)
        strcat(newPath, ".disk_save");
    else
        strcat(newPath, ".ram");

    return newPath;
}

I am not familiar with the Retroarch API. Would it be possible to check if "savefile_directory" stored in the retroarch.cfg file is not NULL and use that instead?

Should the "savefile_directory" not be set, would it be possible to create a temporary file in ram or something? I noticed that there is a "save_format" variable that can be set to -1, would that work in this scenario?

m4xw commented 2 years ago

Generally mupen does not support running on readonly filesystems (exception being the angle branch). Few files need to be dropped, shader cache etc. Can't throw this through RA's io.

m4xw commented 2 years ago

Can consider adding some sanity checks tho

7oxicshadow commented 2 years ago

In my scenario only the rom is stored on a read only file system (NAS) but I can work around it by scripting the files to be copied to a temp ram drive before launching.

I agree that some sanity checks with feedback to the user would be handy.

m4xw commented 2 years ago

tbh the whole thing is quite janky. Currently causes me a bunch of conflicts so will have to rewrite all that media loader shenanigans. Theres a argument to be had to move this to savefiles, but theres some caveats iirc (that i dont fully recall)

m4xw commented 2 years ago

If u want to play around with it, feel free tho. Happy to accept any PR (but like i said, probably needs a rewrite anyway, including all the subsystem stuff)

m4xw commented 2 years ago

Some extra info, before i forget. Those codepaths exists first and foremost for static platforms (i.e. Switch). Subsystem is broken for them, so thats how the naming scheme came to be. For platforms where subsystem works its probably best to allow them as a optional input on selection. Iirc disk save is a copy of the disc with modifications applied so in that case it will be sub-ideal for your use case no matter what I fear

m4xw commented 2 years ago

Disk save / .ram in particular i just smacked in afterwards