Commit 3827daf4 authored by Marius Gerbershagen's avatar Marius Gerbershagen
Browse files

file.d: clean up ecl_stream_open

Previous implementation was messy and contained several race
conditions (multiple open/close operations on the same file, first
checking whether the file exists before opening it). We now always use
a single open call and then optionally do an fdopen later
on (exception being :rename which contains an unavoidable race
condition between checking whether the file exists and then renaming
it later on). Also improve error messages.
parent 2a144148
...@@ -2615,17 +2615,6 @@ safe_close(int f) ...@@ -2615,17 +2615,6 @@ safe_close(int f)
return output; return output;
} }
static FILE *
safe_fopen(const char *filename, const char *mode)
{
const cl_env_ptr the_env = ecl_process_env();
FILE *output;
ecl_disable_interrupts_env(the_env);
output = fopen(filename, mode);
ecl_enable_interrupts_env(the_env);
return output;
}
static FILE * static FILE *
safe_fdopen(int fildes, const char *mode) safe_fdopen(int fildes, const char *mode)
{ {
...@@ -5250,14 +5239,30 @@ FEinvalid_option(cl_object option, cl_object value) ...@@ -5250,14 +5239,30 @@ FEinvalid_option(cl_object option, cl_object value)
FEerror("Invalid value op option ~A: ~A", 2, option, value); FEerror("Invalid value op option ~A: ~A", 2, option, value);
} }
static int
smmode_to_open_flag(enum ecl_smmode smm)
{
switch (smm) {
case ecl_smm_probe:
case ecl_smm_input:
return O_RDONLY;
case ecl_smm_output:
return O_WRONLY;
case ecl_smm_io:
return O_RDWR;
default:
FEerror("Illegal stream mode ~S", 1, ecl_make_fixnum(smm));
}
}
cl_object cl_object
ecl_open_stream(cl_object fn, enum ecl_smmode smm, cl_object if_exists, ecl_open_stream(cl_object fn, enum ecl_smmode smm, cl_object if_exists,
cl_object if_does_not_exist, cl_fixnum byte_size, cl_object if_does_not_exist, cl_fixnum byte_size,
int flags, cl_object external_format) int flags, cl_object external_format)
{ {
cl_object output, file_kind; cl_object output;
int fd; int fd, open_flags = smmode_to_open_flag(smm) | O_BINARY;
bool appending = 0, exists; bool appending = 0;
#if defined(ECL_MS_WINDOWS_HOST) #if defined(ECL_MS_WINDOWS_HOST)
ecl_mode_t mode = _S_IREAD | _S_IWRITE; ecl_mode_t mode = _S_IREAD | _S_IWRITE;
#else #else
...@@ -5267,91 +5272,87 @@ ecl_open_stream(cl_object fn, enum ecl_smmode smm, cl_object if_exists, ...@@ -5267,91 +5272,87 @@ ecl_open_stream(cl_object fn, enum ecl_smmode smm, cl_object if_exists,
remembers the original pathname FN. -- jd 2020-03-27 */ remembers the original pathname FN. -- jd 2020-03-27 */
cl_object filename = si_coerce_to_filename(fn); cl_object filename = si_coerce_to_filename(fn);
char *fname = (char*)filename->base_string.self; char *fname = (char*)filename->base_string.self;
file_kind = si_file_kind(filename, ECL_T);
exists = file_kind != ECL_NIL; if (if_does_not_exist == @':create') {
if (!exists) { open_flags |= O_CREAT;
if (if_does_not_exist == ECL_NIL) return ECL_NIL; if ((smm == ecl_smm_output || smm == ecl_smm_io) &&
if (if_does_not_exist == @':error') FEcannot_open(fn); (if_exists == ECL_NIL || if_exists == @':error' || if_exists == @':rename')) {
if (if_does_not_exist != @':create') open_flags |= O_EXCL;
FEinvalid_option(@':if-does-not-exist', if_does_not_exist); }
fd = safe_open(fname, O_WRONLY|O_CREAT, mode); } else if (if_does_not_exist != ECL_NIL && if_does_not_exist != @':error') {
unlikely_if (fd < 0) FEcannot_open(fn); FEinvalid_option(@':if-does-not-exist', if_does_not_exist);
safe_close(fd); }
fd = -1; if (if_exists == @':rename_and_delete' ||
if_exists == @':new_version' ||
if_exists == @':supersede' ||
if_exists == @':truncate') {
if (smm == ecl_smm_output || smm == ecl_smm_io) {
open_flags |= O_TRUNC;
}
} else if (if_exists == @':append') {
if (smm == ecl_smm_output || smm == ecl_smm_io) {
appending = 1;
}
} else if (if_exists != ECL_NIL &&
if_exists != @':error' &&
if_exists != @':rename' &&
if_exists != @':overwrite') {
FEinvalid_option(@':if-exists', if_exists);
} }
switch (smm) {
case ecl_smm_probe: fd = safe_open(fname, open_flags, mode);
output = ecl_make_file_stream_from_fd(fn, -1, smm, byte_size, flags, external_format); if (fd < 0) {
generic_close(output); if (errno == ENOENT && if_does_not_exist == ECL_NIL) {
return output; return ECL_NIL;
case ecl_smm_input: } else if (errno == EEXIST) {
fd = safe_open(fname, O_RDONLY|O_NONBLOCK, mode); if (if_exists == ECL_NIL) {
unlikely_if (fd < 0) FEcannot_open(fn); safe_close(fd);
break; return ECL_NIL;
case ecl_smm_output: } else if (if_exists == @':error') {
/* For output we could have used O_WRONLY, but this doesn't matter because safe_close(fd);
we fopen with OPEN_RW later anyway. stream opts enforce things. */ FEcannot_open(fn);
case ecl_smm_io: { } else if (if_exists == @':rename') {
if (exists) { safe_close(fd);
if (if_exists == ECL_NIL) return ECL_NIL; fd = ecl_backup_open(fname, smmode_to_open_flag(smm)|O_CREAT, mode);
if (if_exists == @':error') FEcannot_open(fn);
if (if_exists == @':rename') {
fd = ecl_backup_open(fname, O_RDWR|O_CREAT, mode);
unlikely_if (fd < 0) FEcannot_open(fn);
} else if (if_exists == @':rename_and_delete' ||
if_exists == @':new_version' ||
if_exists == @':supersede' ||
if_exists == @':truncate') {
fd = safe_open(fname, O_RDWR|O_TRUNC, mode);
unlikely_if (fd < 0) FEcannot_open(fn);
} else if (if_exists == @':overwrite' || if_exists == @':append') {
fd = safe_open(fname, O_RDWR, mode);
unlikely_if (fd < 0) FEcannot_open(fn); unlikely_if (fd < 0) FEcannot_open(fn);
appending = (if_exists == @':append');
} else {
FEinvalid_option(@':if-exists', if_exists);
} }
} else {
FEcannot_open(fn);
} }
break;
} }
default:
FEerror("Illegal stream mode ~S", 1, ecl_make_fixnum(smm)); if (smm == ecl_smm_probe) {
safe_close(fd);
output = ecl_make_file_stream_from_fd(fn, -1, smm, byte_size, flags, external_format);
generic_close(output);
return output;
} }
if (flags & ECL_STREAM_C_STREAM) { if (flags & ECL_STREAM_C_STREAM) {
FILE *fp = 0; FILE *fp = 0;
/* We do not use fdopen() because Windows seems to have problems with the
* resulting streams. Furthermore, even for output we open with w+ because
* we do not want to overwrite the file. */
switch (smm) { switch (smm) {
case ecl_smm_probe: case ecl_smm_probe:
/* never happens (returns earlier) */ /* never happens (returns earlier) */
case ecl_smm_input: case ecl_smm_input:
if (file_kind == @':fifo') { fp = safe_fdopen(fd, OPEN_R);
fp = safe_fdopen(fd, OPEN_R);
} else {
if (fd >= 0)
safe_close(fd);
fp = safe_fopen(fname, OPEN_R);
}
break; break;
case ecl_smm_output: case ecl_smm_output:
fp = safe_fdopen(fd, OPEN_W);
break;
case ecl_smm_io: case ecl_smm_io:
if (file_kind == @':fifo') { fp = safe_fdopen(fd, OPEN_RW);
fp = safe_fdopen(fd, OPEN_RW);
} else {
if (fd >= 0)
safe_close(fd);
fp = safe_fopen(fname, OPEN_RW);
}
break; break;
default:; default:;
/* never reached (errors earlier) */ /* never reached (errors earlier) */
} }
if (fp == NULL) {
FEcannot_open(fn);
}
output = ecl_make_stream_from_FILE(fn, fp, smm, byte_size, flags, external_format); output = ecl_make_stream_from_FILE(fn, fp, smm, byte_size, flags, external_format);
si_set_buffering_mode(output, byte_size? @':full' : @':line'); si_set_buffering_mode(output, byte_size? @':full' : @':line');
} else { } else {
output = ecl_make_file_stream_from_fd(fn, fd, smm, byte_size, flags, external_format); output = ecl_make_file_stream_from_fd(fn, fd, smm, byte_size, flags, external_format);
} }
output->stream.flags |= ECL_STREAM_MIGHT_SEEK; output->stream.flags |= ECL_STREAM_MIGHT_SEEK;
si_set_finalizer(output, ECL_T); si_set_finalizer(output, ECL_T);
/* Set file pointer to the correct position */ /* Set file pointer to the correct position */
...@@ -5703,7 +5704,7 @@ file_libc_error(cl_object error_type, cl_object stream, ...@@ -5703,7 +5704,7 @@ file_libc_error(cl_object error_type, cl_object stream,
rest = cl_grab_rest_args(args); rest = cl_grab_rest_args(args);
ecl_va_end(args); ecl_va_end(args);
si_signal_simple_error(4, (cl_object)(cl_symbols + ecl_fixnum(error_type)), Cnil, si_signal_simple_error(4, (cl_object)(cl_symbols + ecl_fixnum(error_type)), ECL_NIL,
ecl_make_constant_base_string("~?~%C library explanation: ~A.",-1), ecl_make_constant_base_string("~?~%C library explanation: ~A.",-1),
cl_list(3, ecl_make_constant_base_string(msg,-1), rest, cl_list(3, ecl_make_constant_base_string(msg,-1), rest,
error)); error));
......
...@@ -237,6 +237,10 @@ extern enum ecl_ffi_tag ecl_foreign_type_code(cl_object type); ...@@ -237,6 +237,10 @@ extern enum ecl_ffi_tag ecl_foreign_type_code(cl_object type);
#ifndef O_NONBLOCK #ifndef O_NONBLOCK
#define O_NONBLOCK 0 #define O_NONBLOCK 0
#endif #endif
/* Windows needs to be told explicitely to open files in binary mode */
#ifndef O_BINARY
#define O_BINARY 0
#endif
#define ECL_FILE_STREAM_P(strm) \ #define ECL_FILE_STREAM_P(strm) \
(ECL_ANSI_STREAM_P(strm) && (strm)->stream.mode < ecl_smm_synonym) (ECL_ANSI_STREAM_P(strm) && (strm)->stream.mode < ecl_smm_synonym)
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment