From e69f0ad4504eaa3577c6a337f8a0083b5d467454 Mon Sep 17 00:00:00 2001 From: Carsten Burstedde Date: Tue, 16 Jan 2024 08:34:54 +0100 Subject: [PATCH 1/4] sc_io: add source/sink_destroy_null functions --- src/sc_io.c | 38 ++++++++++++++++++++++++++++++++++++++ src/sc_io.h | 28 +++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/sc_io.c b/src/sc_io.c index fc05ff8b..82e79827 100644 --- a/src/sc_io.c +++ b/src/sc_io.c @@ -102,6 +102,25 @@ sc_io_sink_destroy (sc_io_sink_t * sink) return retval ? SC_IO_ERROR_FATAL : SC_IO_ERROR_NONE; } +int +sc_io_sink_destroy_null (sc_io_sink_t ** sink) +{ + int retval = SC_IO_ERROR_NONE; + + /* pointer to sink pointer must be set */ + SC_ASSERT (sink != NULL); + + /* if sink is still open, close it and NULL the pointer to it */ + if (*sink != NULL) { + retval = sc_io_sink_destroy (*sink); + *sink = NULL; + } + + /* in any case the sink does no longer exist */ + SC_ASSERT (*sink == NULL); + return retval; +} + int sc_io_sink_write (sc_io_sink_t * sink, const void *data, size_t bytes_avail) { @@ -255,6 +274,25 @@ sc_io_source_destroy (sc_io_source_t * source) return retval ? SC_IO_ERROR_FATAL : SC_IO_ERROR_NONE; } +int +sc_io_source_destroy_null (sc_io_source_t ** source) +{ + int retval = SC_IO_ERROR_NONE; + + /* pointer to source pointer must be set */ + SC_ASSERT (source != NULL); + + /* if source is still open, close it and NULL the pointer to it */ + if (*source != NULL) { + retval = sc_io_source_destroy (*source); + *source = NULL; + } + + /* in any case the source does no longer exist */ + SC_ASSERT (*source == NULL); + return retval; +} + int sc_io_source_read (sc_io_source_t * source, void *data, size_t bytes_avail, size_t *bytes_out) diff --git a/src/sc_io.h b/src/sc_io.h index e1c37163..c510141d 100644 --- a/src/sc_io.h +++ b/src/sc_io.h @@ -191,6 +191,19 @@ sc_io_sink_t *sc_io_sink_new (int iotype, int iomode, */ int sc_io_sink_destroy (sc_io_sink_t * sink); +/** Free data sink and NULL the pointer to it. + * Except for the handling of the pointer argument, + * the behavior is the same as for \ref sc_io_sink_destroy. + * \param [in,out] sink Non-NULL pointer to sink pointer. + * The sink pointer may be NULL, in which case + * this function does nothing successfully, + * or a valid \ref sc_io_sink, which is + * passed to \ref sc_io_sink_destroy, and the + * sink pointer is set to NULL afterwards. + * \return 0 on success, nonzero on error. + */ +int sc_io_sink_destroy_null (sc_io_sink_t ** sink); + /** Write data to a sink. Data may be buffered and sunk in a later call. * The internal counters sink->bytes_in and sink->bytes_out are updated. * \param [in,out] sink The sink object to write to. @@ -250,6 +263,19 @@ sc_io_source_t *sc_io_source_new (int iotype, int ioencode, ...); */ int sc_io_source_destroy (sc_io_source_t * source); +/** Free data source and NULL the pointer to it. + * Except for the handling of the pointer argument, + * the behavior is the same as for \ref sc_io_source_destroy. + * \param [in,out] source Non-NULL pointer to source pointer. + * The source pointer may be NULL, in which case + * this function does nothing successfully, + * or a valid \ref sc_io_source, which is + * passed to \ref sc_io_source_destroy, and the + * source pointer is set to NULL afterwards. + * \return 0 on success, nonzero on error. + */ +int sc_io_source_destroy_null (sc_io_source_t ** source); + /** Read data from a source. * The internal counters source->bytes_in and source->bytes_out are updated. * Data is read until the data buffer has not enough room anymore, or source @@ -258,7 +284,7 @@ int sc_io_source_destroy (sc_io_source_t * source); * check its return value to find out. * Returns an error if bytes_out is NULL and less than bytes_avail are read. * \param [in,out] source The source object to read from. - * \param [in] data Data buffer for reading from sink. + * \param [in] data Data buffer for reading from source. * If NULL the output data will be thrown away. * \param [in] bytes_avail Number of bytes available in data buffer. * \param [in,out] bytes_out If not NULL, byte count read into data buffer. From cb5e96aa017b2845df9de8118ced041cc3d757af Mon Sep 17 00:00:00 2001 From: Carsten Burstedde Date: Tue, 16 Jan 2024 08:56:38 +0100 Subject: [PATCH 2/4] sc_io_source: add feof and further checks We remember if we reach the end of input. In this case, any further reading returns 0 bytes. We add a couple documentation items and assertions. --- src/sc_io.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++----- src/sc_io.h | 12 +++++++----- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/sc_io.c b/src/sc_io.c index 82e79827..a8feb7b8 100644 --- a/src/sc_io.c +++ b/src/sc_io.c @@ -213,20 +213,25 @@ sc_io_source_new (int iotype, int ioencode, ...) sc_io_source_t *source; va_list ap; + /* verify preconditions */ SC_ASSERT (0 <= iotype && iotype < SC_IO_TYPE_LAST); SC_ASSERT (0 <= ioencode && ioencode < SC_IO_ENCODE_LAST); + /* initialize members of source object */ source = SC_ALLOC_ZERO (sc_io_source_t, 1); source->iotype = (sc_io_type_t) iotype; source->encode = (sc_io_encode_t) ioencode; + /* there is at least one type-dependent argument */ va_start (ap, ioencode); if (iotype == SC_IO_TYPE_BUFFER) { + /* the source is presented in the form of an array */ source->buffer = va_arg (ap, sc_array_t *); } else if (iotype == SC_IO_TYPE_FILENAME) { const char *filename = va_arg (ap, const char *); + /* open a file on disk by name */ source->file = fopen (filename, "rb"); if (source->file == NULL) { SC_FREE (source); @@ -234,6 +239,7 @@ sc_io_source_new (int iotype, int ioencode, ...) } } else if (iotype == SC_IO_TYPE_FILEFILE) { + /* read from an existing (readable) file object */ source->file = va_arg (ap, FILE *); if (ferror (source->file)) { SC_FREE (source); @@ -245,6 +251,7 @@ sc_io_source_new (int iotype, int ioencode, ...) } va_end (ap); + /* this source can now be called for reading */ return source; } @@ -300,38 +307,74 @@ sc_io_source_read (sc_io_source_t * source, void *data, int retval; size_t bbytes_out; + /* basic input preconditions. It is legal if data is NULL */ + SC_ASSERT (source != NULL); + + /* do nothing also if the end of the file has been reached */ + if (bytes_avail == 0 || source->is_eof) { + if (bytes_out != NULL) { + *bytes_out = 0; + } + return SC_IO_ERROR_NONE; + } + + /* do a regular read */ retval = 0; bbytes_out = 0; + /* switch on the type of source */ if (source->iotype == SC_IO_TYPE_BUFFER) { SC_ASSERT (source->buffer != NULL); + + /* access total byte count theoretically available in input buffer */ bbytes_out = SC_ARRAY_BYTE_ALLOC (source->buffer); SC_ASSERT (bbytes_out >= source->buffer_bytes); + + /* compute how many bytes may be read now on top of the previous ones */ bbytes_out -= source->buffer_bytes; - bbytes_out = SC_MIN (bbytes_out, bytes_avail); + if (bbytes_out == 0) { + /* note end of buffer memory */ + source->is_eof = 1; + } + else { + /* we may be instructed to read less bytes than available */ + bbytes_out = SC_MIN (bbytes_out, bytes_avail); - if (data != NULL) { - memcpy (data, source->buffer->array + source->buffer_bytes, bbytes_out); + /* In the present code we read to the end of the buffer allocation. + * This may not be what we want: we may only read actual elements, + * which may be less. + * TO DO: look into it. */ + + /* copy into output buffer only if that is made available */ + if (data != NULL) { + memcpy (data, source->buffer->array + source->buffer_bytes, bbytes_out); + } + source->buffer_bytes += bbytes_out; } - source->buffer_bytes += bbytes_out; } else if (source->iotype == SC_IO_TYPE_FILENAME || source->iotype == SC_IO_TYPE_FILEFILE) { SC_ASSERT (source->file != NULL); if (data != NULL) { + SC_ASSERT (bytes_avail > 0); bbytes_out = fread (data, 1, bytes_avail, source->file); if (bbytes_out < bytes_avail) { - retval = !feof (source->file) || ferror (source->file); + /* the item count read is short or zero, which is also short */ + retval = !(source->is_eof = feof (source->file)) || + ferror (source->file); } if (retval == SC_IO_ERROR_NONE && source->mirror != NULL) { retval = sc_io_sink_write (source->mirror, data, bbytes_out); } } else { + /* seek now and check for potential end of file next time */ retval = fseek (source->file, (long) bytes_avail, SEEK_CUR); bbytes_out = bytes_avail; } } + + /* process error conditions */ if (retval) { return SC_IO_ERROR_FATAL; } @@ -339,12 +382,14 @@ sc_io_source_read (sc_io_source_t * source, void *data, return SC_IO_ERROR_FATAL; } + /* complete and return on successful operation */ if (bytes_out != NULL) { *bytes_out = bbytes_out; } source->bytes_in += bbytes_out; source->bytes_out += bbytes_out; + /* success! */ return SC_IO_ERROR_NONE; } diff --git a/src/sc_io.h b/src/sc_io.h index c510141d..6d30cb6d 100644 --- a/src/sc_io.h +++ b/src/sc_io.h @@ -142,14 +142,15 @@ typedef struct sc_io_source sc_io_type_t iotype; /**< type of the I/O operation */ sc_io_encode_t encode; /**< encoding of data */ sc_array_t *buffer; /**< buffer for the iotype - SC_IO_TYPE_BUFFER*/ - size_t buffer_bytes; /**< distinguish from array elems */ + \ref SC_IO_TYPE_BUFFER */ + size_t buffer_bytes; /**< distinguish from array elements */ FILE *file; /**< file pointer for iotype unequal to - SC_IO_TYPE_BUFFER */ + \ref SC_IO_TYPE_BUFFER */ size_t bytes_in; /**< input bytes count */ size_t bytes_out; /**< read bytes count */ + int is_eof; /**< Have we reached the end of file? */ sc_io_sink_t *mirror; /**< if activated, a sink to store the - data*/ + data */ sc_array_t *mirror_buffer; /**< if activated, the buffer for the mirror */ } @@ -285,7 +286,8 @@ int sc_io_source_destroy_null (sc_io_source_t ** source); * Returns an error if bytes_out is NULL and less than bytes_avail are read. * \param [in,out] source The source object to read from. * \param [in] data Data buffer for reading from source. - * If NULL the output data will be thrown away. + * If NULL the output data will be ignored + * and we seek forward in the input. * \param [in] bytes_avail Number of bytes available in data buffer. * \param [in,out] bytes_out If not NULL, byte count read into data buffer. * Otherwise, requires to read exactly bytes_avail. From 28bd879b4b384b32a4075ace41bb5dcb29c5c3cf Mon Sep 17 00:00:00 2001 From: Carsten Burstedde Date: Tue, 16 Jan 2024 09:45:19 +0100 Subject: [PATCH 3/4] sc_io_source: do not read beyond end of array Previously we would read all allocated memory of an input buffer, even if the elements require less than this byte size. Changed to read at most the elements present and no longer access undefined memory. --- src/sc_io.c | 24 ++++++++++++++---------- src/sc_io.h | 1 + 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/sc_io.c b/src/sc_io.c index a8feb7b8..4ae17eb3 100644 --- a/src/sc_io.c +++ b/src/sc_io.c @@ -326,24 +326,28 @@ sc_io_source_read (sc_io_source_t * source, void *data, if (source->iotype == SC_IO_TYPE_BUFFER) { SC_ASSERT (source->buffer != NULL); - /* access total byte count theoretically available in input buffer */ - bbytes_out = SC_ARRAY_BYTE_ALLOC (source->buffer); - SC_ASSERT (bbytes_out >= source->buffer_bytes); + /* access available elements by their byte count */ + bbytes_out = source->buffer->elem_count * source->buffer->elem_size; /* compute how many bytes may be read now on top of the previous ones */ - bbytes_out -= source->buffer_bytes; + if (bbytes_out < source->buffer_bytes) { + /* the input buffer has shrunk by side effects: stop reading gracefully */ + bbytes_out = 0; + } + else { + /* we may have some remaining bytes to read */ + bbytes_out -= source->buffer_bytes; + } + + /* check for end of input and read if data is available */ if (bbytes_out == 0) { - /* note end of buffer memory */ + /* register end of available data */ source->is_eof = 1; } else { /* we may be instructed to read less bytes than available */ bbytes_out = SC_MIN (bbytes_out, bytes_avail); - - /* In the present code we read to the end of the buffer allocation. - * This may not be what we want: we may only read actual elements, - * which may be less. - * TO DO: look into it. */ + SC_ASSERT (bbytes_out > 0); /* copy into output buffer only if that is made available */ if (data != NULL) { diff --git a/src/sc_io.h b/src/sc_io.h index 6d30cb6d..beac8224 100644 --- a/src/sc_io.h +++ b/src/sc_io.h @@ -291,6 +291,7 @@ int sc_io_source_destroy_null (sc_io_source_t ** source); * \param [in] bytes_avail Number of bytes available in data buffer. * \param [in,out] bytes_out If not NULL, byte count read into data buffer. * Otherwise, requires to read exactly bytes_avail. + * If this condition is not met, return an error. * \return 0 on success, nonzero on error. */ int sc_io_source_read (sc_io_source_t * source, From 7657b7ee78f445c9f334d0c870efaf3ca66f721f Mon Sep 17 00:00:00 2001 From: Carsten Burstedde Date: Tue, 16 Jan 2024 08:57:39 +0100 Subject: [PATCH 4/4] sc_io_sink: add documentation and early return --- src/sc_io.c | 26 +++++++++++++++++++++++++- src/sc_io.h | 9 +++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/sc_io.c b/src/sc_io.c index 4ae17eb3..dc296ae6 100644 --- a/src/sc_io.c +++ b/src/sc_io.c @@ -43,17 +43,21 @@ sc_io_sink_new (int iotype, int iomode, int ioencode, ...) sc_io_sink_t *sink; va_list ap; + /* verify preconditions */ SC_ASSERT (0 <= iotype && iotype < SC_IO_TYPE_LAST); SC_ASSERT (0 <= iomode && iomode < SC_IO_MODE_LAST); SC_ASSERT (0 <= ioencode && ioencode < SC_IO_ENCODE_LAST); + /* initialize members of sink object */ sink = SC_ALLOC_ZERO (sc_io_sink_t, 1); sink->iotype = (sc_io_type_t) iotype; sink->mode = (sc_io_mode_t) iomode; sink->encode = (sc_io_encode_t) ioencode; + /* there is at least one type-dependent argument */ va_start (ap, ioencode); if (iotype == SC_IO_TYPE_BUFFER) { + /* register the buffer to write to */ sink->buffer = va_arg (ap, sc_array_t *); if (sink->mode == SC_IO_MODE_WRITE) { sc_array_resize (sink->buffer, 0); @@ -62,6 +66,7 @@ sc_io_sink_new (int iotype, int iomode, int ioencode, ...) else if (iotype == SC_IO_TYPE_FILENAME) { const char *filename = va_arg (ap, const char *); + /* open a file on disk by name */ sink->file = fopen (filename, sink->mode == SC_IO_MODE_WRITE ? "wb" : "ab"); if (sink->file == NULL) { @@ -70,6 +75,7 @@ sc_io_sink_new (int iotype, int iomode, int ioencode, ...) } } else if (iotype == SC_IO_TYPE_FILEFILE) { + /* write to existing (writable) object */ sink->file = va_arg (ap, FILE *); if (ferror (sink->file)) { SC_FREE (sink); @@ -81,6 +87,7 @@ sc_io_sink_new (int iotype, int iomode, int ioencode, ...) } va_end (ap); + /* this sink can now be called for writing */ return sink; } @@ -126,21 +133,35 @@ sc_io_sink_write (sc_io_sink_t * sink, const void *data, size_t bytes_avail) { size_t bytes_out; + /* basic output preconditions */ + SC_ASSERT (sink != NULL); + SC_ASSERT (data != NULL || bytes_avail == 0); + + /* do nothing if there is no data requested */ + if (bytes_avail == 0) { + return SC_IO_ERROR_NONE; + } + + /* do a regular write */ bytes_out = 0; + /* switch on the type of sink */ if (sink->iotype == SC_IO_TYPE_BUFFER) { size_t elem_size, new_count; + /* extend the buffer by an even number of elements if necessary */ SC_ASSERT (sink->buffer != NULL); elem_size = sink->buffer->elem_size; new_count = (sink->buffer_bytes + bytes_avail + elem_size - 1) / elem_size; sc_array_resize (sink->buffer, new_count); - /* For a view sufficient size is asserted only in debug mode. */ + /* For a view, sufficient size is asserted only in debug mode. + Therefore, we add an explicit unconditional check. */ if (new_count * elem_size > SC_ARRAY_BYTE_ALLOC (sink->buffer)) { return SC_IO_ERROR_FATAL; } + /* copy new data into the buffer at the proper position */ memcpy (sink->buffer->array + sink->buffer_bytes, data, bytes_avail); sink->buffer_bytes += bytes_avail; bytes_out = bytes_avail; @@ -150,13 +171,16 @@ sc_io_sink_write (sc_io_sink_t * sink, const void *data, size_t bytes_avail) SC_ASSERT (sink->file != NULL); bytes_out = fwrite (data, 1, bytes_avail, sink->file); if (bytes_out != bytes_avail) { + /* a short byte count indicates end of file (not acceptable) or error */ return SC_IO_ERROR_FATAL; } } + /* update internal state and return on successful operation */ sink->bytes_in += bytes_avail; sink->bytes_out += bytes_out; + /* success! */ return SC_IO_ERROR_NONE; } diff --git a/src/sc_io.h b/src/sc_io.h index beac8224..9448c222 100644 --- a/src/sc_io.h +++ b/src/sc_io.h @@ -127,12 +127,13 @@ typedef struct sc_io_sink sc_io_mode_t mode; /**< write semantics */ sc_io_encode_t encode; /**< encoding of data */ sc_array_t *buffer; /**< buffer for the iotype - SC_IO_TYPE_BUFFER*/ - size_t buffer_bytes; /**< distinguish from array elems */ + \ref SC_IO_TYPE_BUFFER */ + size_t buffer_bytes; /**< distinguish from array elements */ FILE *file; /**< file pointer for iotype unequal to - SC_IO_TYPE_BUFFER */ + \ref SC_IO_TYPE_BUFFER */ size_t bytes_in; /**< input bytes count */ size_t bytes_out; /**< written bytes count */ + int is_eof; /**< Have we reached the end of file? */ } sc_io_sink_t; @@ -208,7 +209,7 @@ int sc_io_sink_destroy_null (sc_io_sink_t ** sink); /** Write data to a sink. Data may be buffered and sunk in a later call. * The internal counters sink->bytes_in and sink->bytes_out are updated. * \param [in,out] sink The sink object to write to. - * \param [in] data Data passed into sink. + * \param [in] data Data passed into sink must be non-NULL. * \param [in] bytes_avail Number of data bytes passed in. * \return 0 on success, nonzero on error. */