From c35f98ef588030352b2b7a09312484db3a20ee3c Mon Sep 17 00:00:00 2001 From: Daniel Kochmanski Date: Sun, 6 May 2018 19:28:43 +0200 Subject: [PATCH] ecl_open_stream refactor We rearrange the code and avoid unnecessary file operations with probe. Some nesting is removed. --- src/c/file.d | 125 +++++++++++++++++++++---------------------------- src/h/object.h | 29 +++++------- 2 files changed, 67 insertions(+), 87 deletions(-) diff --git a/src/c/file.d b/src/c/file.d index 227d34c6b..9d06ed462 100755 --- a/src/c/file.d +++ b/src/c/file.d @@ -5060,115 +5060,98 @@ FEinvalid_option(cl_object option, cl_object value) } cl_object -ecl_open_stream(cl_object fn, enum ecl_smmode smm, cl_object if_exists, +ecl_open_stream(cl_object filename, enum ecl_smmode smm, cl_object if_exists, cl_object if_does_not_exist, cl_fixnum byte_size, int flags, cl_object external_format) { cl_object output; int f; + char *fname; + bool appending = 0, exists; #if defined(ECL_MS_WINDOWS_HOST) ecl_mode_t mode = _S_IREAD | _S_IWRITE; #else ecl_mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; #endif - cl_object filename = si_coerce_to_filename(fn); - char *fname = (char*)filename->base_string.self; - bool appending = 0; - bool exists = si_file_kind(filename, ECL_T) != ECL_NIL; - if (smm == ecl_smm_input || smm == ecl_smm_probe) { - if (!exists) { - if (if_does_not_exist == @':error') { - FEcannot_open(fn); - } else if (if_does_not_exist == @':create') { - f = safe_open(fname, O_WRONLY|O_CREAT, mode); - unlikely_if (f < 0) FEcannot_open(fn); - safe_close(f); - } else if (Null(if_does_not_exist)) { - return ECL_NIL; - } else { - FEinvalid_option(@':if-does-not-exist', - if_does_not_exist); - } - } + filename = si_coerce_to_filename(filename); + fname = (char*)filename->base_string.self; + exists = si_file_kind(filename, ECL_T) != ECL_NIL; + if (!exists) { + if (if_does_not_exist == ECL_NIL) return ECL_NIL; + if (if_does_not_exist == @':error') FEcannot_open(filename); + if (if_does_not_exist != @':create') + FEinvalid_option(@':if-does-not-exist', if_does_not_exist); + f = safe_open(fname, O_WRONLY|O_CREAT, mode); + unlikely_if (f < 0) FEcannot_open(filename); + safe_close(f); + } + + switch (smm) { + case ecl_smm_probe: + output = ecl_make_file_stream_from_fd(filename, -1, smm, byte_size, flags, external_format); + generic_close(output); + return output; + case ecl_smm_input: f = safe_open(fname, O_RDONLY, mode); - unlikely_if (f < 0) FEcannot_open(fn); - } else if (smm == ecl_smm_output || smm == ecl_smm_io) { + unlikely_if (f < 0) FEcannot_open(filename); + break; + case ecl_smm_output: + case ecl_smm_io: { int base = (smm == ecl_smm_output)? O_WRONLY : O_RDWR; - if (if_exists == @':new_version' && - if_does_not_exist == @':create') { - exists = 0; - if_does_not_exist = @':create'; - } if (exists) { - if (if_exists == @':error') { - FEcannot_open(fn); - } else if (if_exists == @':rename') { + if (if_exists == ECL_NIL) return ECL_NIL; + if (if_exists == @':error') FEcannot_open(filename); + if (if_exists == @':rename') { f = ecl_backup_open(fname, base|O_CREAT, mode); - unlikely_if (f < 0) FEcannot_open(fn); + unlikely_if (f < 0) FEcannot_open(filename); } else if (if_exists == @':rename_and_delete' || if_exists == @':new_version' || - /* :SUPERSEDE could write to a new file and only upon succesful - close it could replace the file in question. That way we'd - assure a transaction-like behavior. We add :TRUNACTE for - explicit truncate action. */ if_exists == @':supersede' || if_exists == @':truncate') { f = safe_open(fname, base|O_TRUNC, mode); - unlikely_if (f < 0) FEcannot_open(fn); + unlikely_if (f < 0) FEcannot_open(filename); } else if (if_exists == @':overwrite' || if_exists == @':append') { f = safe_open(fname, base, mode); - unlikely_if (f < 0) FEcannot_open(fn); + unlikely_if (f < 0) FEcannot_open(filename); appending = (if_exists == @':append'); - } else if (Null(if_exists)) { - return ECL_NIL; } else { FEinvalid_option(@':if-exists', if_exists); } - } else { - if (if_does_not_exist == @':error') { - FEcannot_open(fn); - } else if (if_does_not_exist == @':create') { - f = safe_open(fname, base | O_CREAT | O_TRUNC, mode); - unlikely_if (f < 0) FEcannot_open(fn); - } else if (Null(if_does_not_exist)) { - return ECL_NIL; - } else { - FEinvalid_option(@':if-does-not-exist', - if_does_not_exist); - } } - } else { + break; + } + default: FEerror("Illegal stream mode ~S", 1, ecl_make_fixnum(smm)); } if (flags & ECL_STREAM_C_STREAM) { FILE *fp = 0; safe_close(f); - /* 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. */ + /* 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) { case ecl_smm_probe: - case ecl_smm_input: fp = safe_fopen(fname, OPEN_R); break; + /* never happens (returns earlier) */ + case ecl_smm_input: + fp = safe_fopen(fname, OPEN_R); + break; case ecl_smm_output: - case ecl_smm_io: fp = safe_fopen(fname, OPEN_RW); break; - default:; /* never reached */ + case ecl_smm_io: + fp = safe_fopen(fname, OPEN_RW); + break; + default:; + /* never reached (errors earlier) */ } - output = ecl_make_stream_from_FILE(fn, fp, smm, byte_size, flags, - external_format); + output = ecl_make_stream_from_FILE(filename, fp, smm, byte_size, flags, external_format); si_set_buffering_mode(output, byte_size? @':full' : @':line'); } else { - output = ecl_make_file_stream_from_fd(fn, f, smm, byte_size, flags, - external_format); - } - if (smm == ecl_smm_probe) { - cl_close(1, output); - } else { - output->stream.flags |= ECL_STREAM_MIGHT_SEEK; - si_set_finalizer(output, ECL_T); - /* Set file pointer to the correct position */ - ecl_file_position_set(output, appending? ECL_NIL : ecl_make_fixnum(0)); + output = ecl_make_file_stream_from_fd(filename, f, smm, byte_size, flags, external_format); } + + output->stream.flags |= ECL_STREAM_MIGHT_SEEK; + si_set_finalizer(output, ECL_T); + /* Set file pointer to the correct position */ + ecl_file_position_set(output, appending? ECL_NIL : ecl_make_fixnum(0)); return output; } diff --git a/src/h/object.h b/src/h/object.h index 1a5412c63..73e576b1a 100644 --- a/src/h/object.h +++ b/src/h/object.h @@ -466,13 +466,12 @@ struct ecl_array { /* array header */ struct ecl_vector { /* vector header */ /* adjustable flag */ /* has-fill-pointer flag */ - _ECL_HDR2(elttype,flags); /* array element type, has fill ptr, adjustable-p */ + _ECL_HDR2(elttype,flags); /* array element type, has fill ptr, adjustable-p */ cl_object displaced; /* displaced */ cl_index dim; /* dimension */ cl_index fillp; /* fill pointer */ - /* For simple vectors, */ - /* v_fillp is equal to v_dim. */ - union ecl_array_data self; /* pointer to the vector */ + /* For simple vectors, fillp is equal to dim. */ + union ecl_array_data self; /* pointer to the vector */ byte offset; }; @@ -495,11 +494,9 @@ struct ecl_string { /* string header */ /* has-fill-pointer flag */ _ECL_HDR2(elttype,flags); /* array element type, has fill ptr, adjustable-p */ cl_object displaced; /* displaced */ - cl_index dim; /* dimension */ - /* string length */ + cl_index dim; /* dimension, string length */ cl_index fillp; /* fill pointer */ - /* For simple strings, */ - /* st_fillp is equal to st_dim-1. */ + /* For simple strings, fillp is equal to dim-1. */ ecl_character *self; /* pointer to the string */ }; #endif @@ -513,15 +510,15 @@ struct ecl_string { /* string header */ enum ecl_smmode { /* stream mode */ ecl_smm_input, /* input */ - ecl_smm_input_file, /* input */ + ecl_smm_input_file, /* input */ ecl_smm_output, /* output */ ecl_smm_output_file, /* output */ - ecl_smm_io, /* input-output */ - ecl_smm_io_file, /* input-output */ - ecl_smm_synonym, /* synonym */ - ecl_smm_broadcast, /* broadcast */ + ecl_smm_io, /* input-output */ + ecl_smm_io_file, /* input-output */ + ecl_smm_synonym, /* synonym */ + ecl_smm_broadcast, /* broadcast */ ecl_smm_concatenated, /* concatenated */ - ecl_smm_two_way, /* two way */ + ecl_smm_two_way, /* two way */ ecl_smm_echo, /* echo */ ecl_smm_string_input, /* string input */ ecl_smm_string_output, /* string output */ @@ -529,10 +526,10 @@ enum ecl_smmode { /* stream mode */ #if defined(ECL_WSOCK) ecl_smm_input_wsock, /* input socket (Win32) */ ecl_smm_output_wsock, /* output socket (Win32) */ - ecl_smm_io_wsock, /* input/output socket (Win32) */ + ecl_smm_io_wsock, /* input/output socket (Win32) */ #endif #if defined(ECL_MS_WINDOWS_HOST) - ecl_smm_io_wcon, /* windows console (Win32) */ + ecl_smm_io_wcon, /* windows console (Win32) */ #endif ecl_smm_sequence_input, /* sequence input */ ecl_smm_sequence_output /* sequence output */