Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kernel: fix SaveWorkspace to return false in case of an error, and true only if successful #2674

Merged
merged 1 commit into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/saveload.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -575,6 +575,7 @@ Obj SaveWorkspace( Obj fname )

#else
Obj fullname;
Obj result;

if (!IsStringConv(fname))
ErrorQuit("usage: SaveWorkspace( <filename> )",0,0);
Expand All @@ -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 */
Expand All @@ -607,7 +610,7 @@ Obj SaveWorkspace( Obj fname )
/* Restore situation by calling all post-save methods */
ModulesPostSave();

return True;
return result;
#endif
}

Expand Down Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions tst/testbugfix/2018-08-01-SaveWorkspace.tst
Original file line number Diff line number Diff line change
@@ -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