From 9a4110f3c205107a3a48a0e48b760abf3cd1f3bc Mon Sep 17 00:00:00 2001 From: David Oberhollenzer Date: Fri, 9 Jun 2023 13:43:50 +0200 Subject: libio: eliminate direct access of the interal buffer Instead, go through helper functions, which in a next step can be moved inside the implementation. Signed-off-by: David Oberhollenzer --- include/io/istream.h | 63 +++++++++++++++++++++++++++++++++++++++ lib/io/src/get_line.c | 22 +++++++------- lib/io/src/istream.c | 76 ++++++++++++++++++++++++----------------------- lib/io/src/xfrm/istream.c | 34 +++++++++++---------- lib/tar/src/iterator.c | 68 ++++++++++++++++++++++++++---------------- 5 files changed, 175 insertions(+), 88 deletions(-) diff --git a/include/io/istream.h b/include/io/istream.h index 9a8257f..06e8a3b 100644 --- a/include/io/istream.h +++ b/include/io/istream.h @@ -115,6 +115,69 @@ SQFS_INLINE const char *istream_get_filename(istream_t *strm) return strm->get_filename(strm); } +/** + * @brief Peek into the data buffered in an istream + * + * @memberof istream_t + * + * If the internal buffer is empty, the function tries to fetch more, which can + * block, and returns a positive return code if there is no more data to be + * read. Since this and other functions can alter the buffer pointer and + * contents, do not store the pointers returned here across function calls. + * + * Higher level functions like @ref istream_read, providing a Unix read() style + * API are built on top of this primitive. + * + * @param strm A pointer to an istream_t implementation. + * @param out Returns a pointer into an internal buffer on success. + * @param size Returns the number of bytes available in the buffer. + * + * @return Zero on success, a negative error code on failure, + * a postive number on EOF. + */ +SQFS_INLINE int istream_get_buffered_data(istream_t *strm, const sqfs_u8 **out, + size_t *size) +{ + if (strm->buffer_used == 0) { + int ret = strm->precache(strm); + if (ret) + return ret; + if (strm->buffer_used == 0) + return 1; + } + + *out = strm->buffer; + *size = strm->buffer_used; + return 0; +} + +/** + * @brief Mark a section of the internal buffer of an istream as used + * + * @memberof istream_t + * + * This marks the first `count` bytes of the internal buffer as used, + * forcing @ref istream_get_buffered_data to return data afterwards + * and potentially to load more data. + * + * @param strm A pointer to an istream_t implementation. + * @param count The number of bytes used up. + * + * @return Zero on success, a negative error code on failure. + */ +SQFS_INLINE int istream_advance_buffer(istream_t *strm, size_t count) +{ + if (count >= strm->buffer_used) { + strm->buffer += strm->buffer_used; + strm->buffer_used = 0; + return strm->precache(strm); + } + + strm->buffer += count; + strm->buffer_used -= count; + return 0; +} + /** * @brief Skip over a number of bytes in an input stream. * diff --git a/lib/io/src/get_line.c b/lib/io/src/get_line.c index c764d04..a939a6c 100644 --- a/lib/io/src/get_line.c +++ b/lib/io/src/get_line.c @@ -46,12 +46,14 @@ int istream_get_line(istream_t *strm, char **out, for (;;) { bool have_line = false; - size_t i, count; + size_t i, count, avail; + const sqfs_u8 *ptr; + int ret; - if (istream_precache(strm)) + ret = istream_get_buffered_data(strm, &ptr, &avail); + if (ret < 0) goto fail_free; - - if (strm->buffer_used == 0) { + if (ret > 0) { if (line_len == 0) goto out_eof; @@ -62,12 +64,12 @@ int istream_get_line(istream_t *strm, char **out, goto out_eof; } - for (i = 0; i < strm->buffer_used; ++i) { - if (strm->buffer[i] == '\n') + for (i = 0; i < avail; ++i) { + if (ptr[i] == '\n') break; } - if (i < strm->buffer_used) { + if (i < avail) { count = i++; have_line = true; } else { @@ -79,12 +81,12 @@ int istream_get_line(istream_t *strm, char **out, goto fail_errno; line = new; - memcpy(line + line_len, strm->buffer, count); + memcpy(line + line_len, ptr, count); line_len += count; line[line_len] = '\0'; - strm->buffer += i; - strm->buffer_used -= i; + if (istream_advance_buffer(strm, i)) + goto fail_free; if (have_line) { if (line_len > 0 && line[line_len - 1] == '\r') diff --git a/lib/io/src/istream.c b/lib/io/src/istream.c index 0739fd6..6171f2c 100644 --- a/lib/io/src/istream.c +++ b/lib/io/src/istream.c @@ -10,30 +10,32 @@ sqfs_s32 istream_read(istream_t *strm, void *data, size_t size) { sqfs_s32 total = 0; - size_t diff; if (size > 0x7FFFFFFF) size = 0x7FFFFFFF; while (size > 0) { - if (strm->buffer_used == 0) { - if (istream_precache(strm)) - return -1; + const sqfs_u8 *ptr; + size_t diff; + int ret; - if (strm->buffer_used == 0) - break; - } + ret = istream_get_buffered_data(strm, &ptr, &diff); + if (ret > 0) + break; + if (ret < 0) + return ret; - diff = strm->buffer_used; if (diff > size) diff = size; - memcpy(data, strm->buffer, diff); + memcpy(data, ptr, diff); data = (char *)data + diff; - strm->buffer += diff; - strm->buffer_used -= diff; size -= diff; total += diff; + + ret = istream_advance_buffer(strm, diff); + if (ret) + return -1; } return total; @@ -41,27 +43,27 @@ sqfs_s32 istream_read(istream_t *strm, void *data, size_t size) int istream_skip(istream_t *strm, sqfs_u64 size) { - size_t diff; - while (size > 0) { - if (strm->buffer_used == 0) { - if (istream_precache(strm)) - return -1; - - if (strm->buffer_used == 0) { - fprintf(stderr, "%s: unexpected end-of-file\n", - strm->get_filename(strm)); - return -1; - } + const sqfs_u8 *ptr; + size_t diff; + int ret; + + ret = istream_get_buffered_data(strm, &ptr, &diff); + if (ret < 0) + return ret; + if (ret > 0) { + fprintf(stderr, "%s: unexpected end-of-file\n", + strm->get_filename(strm)); + return -1; } - diff = strm->buffer_used; if ((sqfs_u64)diff > size) diff = size; - strm->buffer += diff; - strm->buffer_used -= diff; size -= diff; + + if (istream_advance_buffer(strm, diff)) + return -1; } return 0; @@ -70,31 +72,31 @@ int istream_skip(istream_t *strm, sqfs_u64 size) sqfs_s32 istream_splice(istream_t *in, ostream_t *out, sqfs_u32 size) { sqfs_s32 total = 0; - size_t diff; if (size > 0x7FFFFFFF) size = 0x7FFFFFFF; while (size > 0) { - if (in->buffer_used == 0) { - if (istream_precache(in)) - return -1; + const sqfs_u8 *ptr; + size_t diff; + int ret; - if (in->buffer_used == 0) - break; - } + ret = istream_get_buffered_data(in, &ptr, &diff); + if (ret < 0) + return ret; + if (ret > 0) + break; - diff = in->buffer_used; if (diff > size) diff = size; - if (ostream_append(out, in->buffer, diff)) + if (ostream_append(out, ptr, diff)) + return -1; + if (istream_advance_buffer(in, diff)) return -1; - in->buffer += diff; - in->buffer_used -= diff; - size -= diff; total += diff; + size -= diff; } return total; diff --git a/lib/io/src/xfrm/istream.c b/lib/io/src/xfrm/istream.c index 152563f..a89ac3d 100644 --- a/lib/io/src/xfrm/istream.c +++ b/lib/io/src/xfrm/istream.c @@ -18,7 +18,7 @@ typedef struct istream_xfrm_t { static int xfrm_precache(istream_t *base) { istream_xfrm_t *xfrm = (istream_xfrm_t *)base; - int ret; + int ret, sret; if (base->eof) return 0; @@ -34,20 +34,26 @@ static int xfrm_precache(istream_t *base) base->buffer = xfrm->uncompressed; - ret = istream_precache(xfrm->wrapped); - if (ret != 0) - return ret; - for (;;) { sqfs_u32 in_off = 0, out_off = base->buffer_used; int mode = XFRM_STREAM_FLUSH_NONE; + const sqfs_u8 *ptr; + size_t avail; + + ret = istream_precache(xfrm->wrapped); + if (ret != 0) + return ret; - if (xfrm->wrapped->eof) + ret = istream_get_buffered_data(xfrm->wrapped, &ptr, &avail); + if (ret < 0) + return ret; + if (ret > 0) { mode = XFRM_STREAM_FLUSH_FULL; + avail = 0; + } ret = xfrm->xfrm->process_data(xfrm->xfrm, - xfrm->wrapped->buffer, - xfrm->wrapped->buffer_used, + ptr, avail, xfrm->uncompressed + out_off, BUFSZ - out_off, &in_off, &out_off, mode); @@ -59,17 +65,15 @@ static int xfrm_precache(istream_t *base) } base->buffer_used = out_off; - xfrm->wrapped->buffer += in_off; - xfrm->wrapped->buffer_used -= in_off; + + sret = istream_advance_buffer(xfrm->wrapped, in_off); + if (sret != 0) + return sret; if (ret == XFRM_STREAM_BUFFER_FULL || out_off >= BUFSZ) break; - ret = istream_precache(xfrm->wrapped); - if (ret != 0) - return ret; - - if (xfrm->wrapped->eof && xfrm->wrapped->buffer_used == 0) { + if (mode == XFRM_STREAM_FLUSH_FULL) { if (base->buffer_used == 0) base->eof = true; break; diff --git a/lib/tar/src/iterator.c b/lib/tar/src/iterator.c index 85937c8..2914e3a 100644 --- a/lib/tar/src/iterator.c +++ b/lib/tar/src/iterator.c @@ -73,25 +73,6 @@ static bool is_sparse_region(const tar_iterator_t *tar, sqfs_u64 *count) return true; } -static int data_available(tar_iterator_t *tar, sqfs_u64 want, sqfs_u64 *out) -{ - if (want > tar->stream->buffer_used) { - if (istream_precache(tar->stream)) { - tar->state = SQFS_ERROR_IO; - return -1; - } - - if (tar->stream->buffer_used == 0 && tar->stream->eof) { - tar->state = SQFS_ERROR_CORRUPTED; - return -1; - } - } - - *out = tar->stream->buffer_used <= want ? - tar->stream->buffer_used : want; - return 0; -} - /*****************************************************************************/ static const char *strm_get_filename(istream_t *strm) @@ -108,8 +89,10 @@ static int strm_precache(istream_t *strm) goto out_eof; if (!tar->parent->last_sparse) { - tar->parent->stream->buffer += tar->parent->last_chunk; - tar->parent->stream->buffer_used -= tar->parent->last_chunk; + int ret = istream_advance_buffer(tar->parent->stream, + tar->parent->last_chunk); + if (ret != 0) + return ret; tar->parent->record_size -= tar->parent->last_chunk; } @@ -118,6 +101,8 @@ static int strm_precache(istream_t *strm) goto out_eof; tar->parent->last_sparse = is_sparse_region(tar->parent, &diff); + if (diff == 0) + goto out_eof; if (tar->parent->last_sparse) { if (diff > sizeof(tar->buffer)) @@ -126,15 +111,40 @@ static int strm_precache(istream_t *strm) strm->buffer = tar->buffer; memset(tar->buffer, 0, diff); } else { - if (data_available(tar->parent, diff, &diff)) - goto out_eof; + const sqfs_u8 *ptr; + size_t avail; + int ret; + + for (int i = 0; i < 2; ++i) { + ret = istream_get_buffered_data(tar->parent->stream, + &ptr, &avail); + if (ret > 0) + goto fail_borked; + if (ret < 0) + goto fail_io; + + if (diff <= avail) + break; + ret = istream_precache(tar->parent->stream); + if (ret) + goto fail_io; + } - strm->buffer = tar->parent->stream->buffer; + if (diff > avail) + diff = avail; + + strm->buffer = ptr; } strm->buffer_used = diff; tar->parent->last_chunk = diff; return 0; +fail_borked: + tar->parent->state = SQFS_ERROR_CORRUPTED; + return -1; +fail_io: + tar->parent->state = SQFS_ERROR_IO; + return -1; out_eof: strm->eof = true; strm->buffer_used = 0; @@ -347,6 +357,8 @@ dir_iterator_t *tar_open_stream(istream_t *strm) tar_iterator_t *tar = calloc(1, sizeof(*tar)); dir_iterator_t *it = (dir_iterator_t *)tar; xfrm_stream_t *xfrm = NULL; + const sqfs_u8 *ptr; + size_t size; int ret; if (tar == NULL) @@ -365,11 +377,15 @@ dir_iterator_t *tar_open_stream(istream_t *strm) if (ret != 0) goto out_strm; - ret = tar_probe(strm->buffer, strm->buffer_used); + ret = istream_get_buffered_data(strm, &ptr, &size); + if (ret != 0) + goto out_strm; + + ret = tar_probe(ptr, size); if (ret > 0) goto out_strm; - ret = xfrm_compressor_id_from_magic(strm->buffer, strm->buffer_used); + ret = xfrm_compressor_id_from_magic(ptr, size); if (ret <= 0) goto out_strm; -- cgit v1.2.3