Skip to content

Commit

Permalink
kernel: signal file open errors in a consistent way (#5634)
Browse files Browse the repository at this point in the history
... by returning -1 in SyFopen, even for the case were we run out of
internal file buffers.

Then adjust the test suite check for this situation accordingly.

This fixes intermittent failures in this test on macOS and OpenBSD: for
both of these we run out of OS file descriptors before we run out of
GAP file buffers. But that situation was previously handled differently:
`OutputTextFile` just returns `fail` for most errors opening a file,
just in the one case were we run out of internal buffers is an actual
error raised.

Besides fixing a portability problem with the test suite, I also think
this is the "right" thing overall, in terms of overall consistency.
  • Loading branch information
fingolfin authored Feb 12, 2024
1 parent 1211785 commit deada08
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
1 change: 0 additions & 1 deletion src/sysfiles.c
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,6 @@ Int SyFopen(const Char * name, const Char * mode, BOOL transparent_compress)

if ( fid == ARRAY_SIZE(syBuf) ) {
HashUnlock(&syBuf);
ErrorReturnVoid("Too many open files (internal file descriptor limit reached)", 0, 0, "you can 'return;'");
return (Int)-1;
}

Expand Down
6 changes: 4 additions & 2 deletions tst/testinstall/streams.tst
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,11 @@ Error, Print formatting status must be true or false
# too many open files
gap> streams := [ ];;
gap> for i in [ 1 .. 300 ] do
> Add( streams, OutputTextFile( fname, false ) );
> stream := OutputTextFile( fname, false );
> Assert(0, stream <> fail);
> Add( streams, stream );
> od;;
Error, Too many open files (internal file descriptor limit reached)
Error, Assertion failure
gap> Perform( streams, CloseStream );
gap> RemoveFile(fname);
true
Expand Down

0 comments on commit deada08

Please sign in to comment.