shithub: choc

Info  •  Files  •  Log  •  Branches

doom: Correctly handle savegame fopen() failure.

If the savegame directory is unwriteable we should exit with an error -
the previous behavior caused an infinite loop of attempting to open the
same file repeatedly. Because the player may value the progress they
have made, attempt to write the savegame to a temporary directory first
before exiting with an error.

This fixes #659. Thanks to @terrorcide for the bug report.

--- a/src/doom/g_game.c

+++ b/src/doom/g_game.c

@@ -1611,59 +1611,78 @@

 { 

     char *savegame_file;

     char *temp_savegame_file;

+    char *recovery_savegame_file;

+    recovery_savegame_file = NULL;

     temp_savegame_file = P_TempSaveGameFile();

     savegame_file = P_SaveGameFile(savegameslot);

     // Open the savegame file for writing.  We write to a temporary file

     // and then rename it at the end if it was successfully written.

-    // This prevents an existing savegame from being overwritten by 

+    // This prevents an existing savegame from being overwritten by

     // a corrupted one, or if a savegame buffer overrun occurs.

-

     save_stream = fopen(temp_savegame_file, "wb");

     if (save_stream == NULL)

     {

-        return;

+        // Failed to save the game, so we're going to have to abort. But

+        // to be nice, save to somewhere else before we call I_Error().

+        recovery_savegame_file = M_TempFile("recovery.dsg");

+        save_stream = fopen(recovery_savegame_file, "wb");

+        if (save_stream == NULL)

+        {

+            I_Error("Failed to open either '%s' or '%s' to write savegame.",

+                    temp_savegame_file, recovery_savegame_file);

+        }

     }

     savegame_error = false;

     P_WriteSaveGameHeader(savedescription);

- 

-    P_ArchivePlayers (); 

-    P_ArchiveWorld (); 

-    P_ArchiveThinkers (); 

-    P_ArchiveSpecials (); 

-	 

+

+    P_ArchivePlayers ();

+    P_ArchiveWorld ();

+    P_ArchiveThinkers ();

+    P_ArchiveSpecials ();

+

     P_WriteSaveGameEOF();

-	 

-    // Enforce the same savegame size limit as in Vanilla Doom, 

+

+    // Enforce the same savegame size limit as in Vanilla Doom,

     // except if the vanilla_savegame_limit setting is turned off.

     if (vanilla_savegame_limit && ftell(save_stream) > SAVEGAMESIZE)

     {

-        I_Error ("Savegame buffer overrun");

+        I_Error("Savegame buffer overrun");

     }

-    

+

     // Finish up, close the savegame file.

     fclose(save_stream);

+    if (recovery_savegame_file != NULL)

+    {

+        // We failed to save to the normal location, but we wrote a

+        // recovery file to the temp directory. Now we can bomb out

+        // with an error.

+        I_Error("Failed to open savegame file '%s' for writing.\n"

+                "But your game has been saved to '%s' for recovery.",

+                temp_savegame_file, recovery_savegame_file);

+    }

+

     // Now rename the temporary savegame file to the actual savegame

     // file, overwriting the old savegame if there was one there.

     remove(savegame_file);

     rename(temp_savegame_file, savegame_file);

-    

-    gameaction = ga_nothing; 

+

+    gameaction = ga_nothing;

     M_StringCopy(savedescription, "", sizeof(savedescription));

     players[consoleplayer].message = DEH_String(GGSAVED);

     // draw the pattern into the back screen

-    R_FillBackScreen ();	

-} 

+    R_FillBackScreen ();

+}

 //