From 6712e3382de75a30a1a8a3fe9ea6958ed1f5bfe4 Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 1 Aug 2018 13:15:44 +0200
Subject: [PATCH] kernel: improve error handling in SaveWorkspace

If we fail to open the output file (e.g. because the target location does not
exist or is not writeable for some other reason), then return false instead of
true.

Also, terminate all error messages with a new line.

Fixes #2673
---
 src/saveload.c                              | 11 +++++++----
 tst/testbugfix/2018-08-01-SaveWorkspace.tst |  7 +++++++
 2 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 tst/testbugfix/2018-08-01-SaveWorkspace.tst

diff --git a/src/saveload.c b/src/saveload.c
index 4b0f75c2ef..1e180891d5 100644
--- a/src/saveload.c
+++ b/src/saveload.c
@@ -53,13 +53,13 @@ static Int OpenForSave( Obj fname )
 {
   if (SaveFile != -1)
     {
-      Pr("Already saving",0L,0L);
+      Pr("Already saving\n",0L,0L);
       return 1;
     }
   SaveFile = SyFopen(CSTR_STRING(fname), "wb");
   if (SaveFile == -1)
     {
-      Pr("Couldn't open file %s to save workspace",
+      Pr("Couldn't open file %s to save workspace\n",
 	 (UInt)CSTR_STRING(fname),0L);
       return 1;
     }
@@ -575,6 +575,7 @@ Obj SaveWorkspace( Obj fname )
 
 #else
   Obj fullname;
+  Obj result;
 
   if (!IsStringConv(fname))
     ErrorQuit("usage: SaveWorkspace( <filename> )",0,0);
@@ -592,8 +593,10 @@ Obj SaveWorkspace( Obj fname )
   CallbackForAllBags( AddSaveIndex );
 
   /* Now do the work */
+  result = Fail;
   if (!OpenForSave( fullname ))
     {
+      result = True;
       WriteSaveHeader();
       SaveCStr("Bag data");
       SortHandlers( 1 ); /* Sort by address to speed up CookieOfHandler */
@@ -607,7 +610,7 @@ Obj SaveWorkspace( Obj fname )
   /* Restore situation by calling all post-save methods */
   ModulesPostSave();
 
-  return True;
+  return result;
 #endif
 }
 
@@ -636,7 +639,7 @@ Obj FuncSaveWorkspace(Obj self, Obj filename )
 void LoadWorkspace( Char * fname )
 {
 #ifndef USE_GASMAN
-  Pr("LoadWorkspace is only supported when GASMAN is in use",0,0);
+  Pr("LoadWorkspace is only supported when GASMAN is in use\n",0,0);
 
 #else
   UInt nGlobs, nBags, i, maxSize;
diff --git a/tst/testbugfix/2018-08-01-SaveWorkspace.tst b/tst/testbugfix/2018-08-01-SaveWorkspace.tst
new file mode 100644
index 0000000000..2de61512c5
--- /dev/null
+++ b/tst/testbugfix/2018-08-01-SaveWorkspace.tst
@@ -0,0 +1,7 @@
+#
+# SaveWorkspace used to return 'true' even if it failed to open the
+# output file. Also, the error message was not trailed by a newline.
+# See https://github.com/gap-system/gap/issues/2673
+gap> SaveWorkspace("fantasy-dir/test");
+Couldn't open file fantasy-dir/test to save workspace
+fail