From e3ef871d6a80d72db02c9ab1ef492e8f58c2ddeb Mon Sep 17 00:00:00 2001 From: David Oberhollenzer Date: Tue, 16 Jul 2019 21:02:58 +0200 Subject: cleanup: move error handling into read_retry If read_retry fails to read the expected amount of data (EOF or otherwise), it is almost always an error. This commit renames read_retry to read_data and moves error handling into the function, making a lot of error handling code redundant. Signed-off-by: David Oberhollenzer --- lib/Makemodule.am | 2 +- lib/comp/compressor.c | 13 ++----------- lib/sqfs/data_reader.c | 16 +++------------- lib/sqfs/data_writer.c | 31 ++++++------------------------- lib/sqfs/frag_reader.c | 27 +++++++-------------------- lib/sqfs/id_table_read.c | 17 ++++------------- lib/sqfs/meta_reader.c | 22 ++++------------------ lib/sqfs/read_super.c | 12 +----------- lib/tar/read_header.c | 30 +++++------------------------- lib/tar/read_sparse_map_old.c | 13 ++++--------- lib/tar/skip.c | 12 +----------- lib/util/read_data.c | 30 ++++++++++++++++++++++++++++++ lib/util/read_retry.c | 27 --------------------------- 13 files changed, 68 insertions(+), 184 deletions(-) create mode 100644 lib/util/read_data.c delete mode 100644 lib/util/read_retry.c (limited to 'lib') diff --git a/lib/Makemodule.am b/lib/Makemodule.am index 710db78..7e6ca82 100644 --- a/lib/Makemodule.am +++ b/lib/Makemodule.am @@ -37,7 +37,7 @@ libsquashfs_a_SOURCES += include/data_writer.h include/frag_reader.h libsquashfs_a_SOURCES += include/data_reader.h lib/sqfs/data_reader.c libutil_a_SOURCES = lib/util/canonicalize_name.c lib/util/write_data.c -libutil_a_SOURCES += lib/util/read_retry.c include/util.h +libutil_a_SOURCES += lib/util/read_data.c include/util.h libutil_a_SOURCES += lib/util/print_version.c lib/util/mkdir_p.c libutil_a_SOURCES += lib/util/str_table.c include/str_table.h libutil_a_SOURCES += lib/util/dirstack.c lib/util/padd_file.c diff --git a/lib/comp/compressor.c b/lib/comp/compressor.c index 96bc3b1..b92462e 100644 --- a/lib/comp/compressor.c +++ b/lib/comp/compressor.c @@ -74,18 +74,9 @@ int generic_write_options(int fd, const void *data, size_t size) int generic_read_options(int fd, void *data, size_t size) { uint8_t buffer[size + 2]; - ssize_t ret; - ret = read_retry(fd, buffer, sizeof(buffer)); - - if (ret < 0) { - perror("reading compressor options"); - return -1; - } - - if ((size_t)ret < sizeof(buffer)) { - fputs("reading compressor options: unexpected end of file\n", - stderr); + if (read_data("reading compressor options", + fd, buffer, sizeof(buffer))) { return -1; } diff --git a/lib/sqfs/data_reader.c b/lib/sqfs/data_reader.c index 7f857b2..96ccb39 100644 --- a/lib/sqfs/data_reader.c +++ b/lib/sqfs/data_reader.c @@ -110,13 +110,9 @@ int data_reader_dump_file(data_reader_t *data, file_info_t *fi, int outfd, if (bs == 0) { memset(data->buffer, 0, unpackedsz); compressed = false; - } else { - ret = read_retry(data->sqfsfd, data->buffer, bs); - if (ret < 0) - goto fail_rd; - - if ((size_t)ret < bs) - goto fail_trunc; + } else if (read_data("reading data block", + data->sqfsfd, data->buffer, bs)) { + return -1; } if (compressed) { @@ -163,12 +159,6 @@ fail_sparse: fail_seek: perror("seek on squashfs"); return -1; -fail_rd: - perror("reading from squashfs"); - return -1; -fail_trunc: - fputs("reading from squashfs: unexpected end of file\n", stderr); - return -1; fail_bs: fputs("found compressed block larger than block size\n", stderr); return -1; diff --git a/lib/sqfs/data_writer.c b/lib/sqfs/data_writer.c index eebd2a4..64e27fa 100644 --- a/lib/sqfs/data_writer.c +++ b/lib/sqfs/data_writer.c @@ -160,7 +160,6 @@ int write_data_from_fd(data_writer_t *data, file_info_t *fi, int infd, int flags) { uint64_t count; - ssize_t ret; size_t diff; if ((flags & DW_ALLIGN_DEVBLK) && allign_file(data) != 0) @@ -174,11 +173,8 @@ int write_data_from_fd(data_writer_t *data, file_info_t *fi, diff = count > (uint64_t)data->super->block_size ? data->super->block_size : count; - ret = read_retry(infd, data->block, diff); - if (ret < 0) - goto fail_read; - if ((size_t)ret < diff) - goto fail_trunc; + if (read_data(fi->input_file, infd, data->block, diff)) + return -1; if (flush_data_block(data, diff, fi, flags)) return -1; @@ -188,12 +184,6 @@ int write_data_from_fd(data_writer_t *data, file_info_t *fi, return -1; return 0; -fail_read: - perror(fi->input_file); - return -1; -fail_trunc: - fprintf(stderr, "%s: truncated read\n", fi->input_file); - return -1; } int write_data_from_fd_condensed(data_writer_t *data, file_info_t *fi, @@ -202,7 +192,6 @@ int write_data_from_fd_condensed(data_writer_t *data, file_info_t *fi, size_t start, count, diff; sparse_map_t *m; uint64_t offset; - ssize_t ret; if ((flags & DW_ALLIGN_DEVBLK) && allign_file(data) != 0) return -1; @@ -246,12 +235,10 @@ int write_data_from_fd_condensed(data_writer_t *data, file_info_t *fi, if (start + count > diff) count = diff - start; - ret = read_retry(infd, (char *)data->block + start, - count); - if (ret < 0) - goto fail_read; - if ((size_t)ret < count) - goto fail_trunc; + if (read_data(fi->input_file, infd, + (char *)data->block + start, count)) { + return -1; + } map = map->next; } @@ -273,12 +260,6 @@ fail_map: "%s: sparse file map is unordered or self overlapping\n", fi->input_file); return -1; -fail_read: - perror(fi->input_file); - return -1; -fail_trunc: - fprintf(stderr, "%s: truncated read\n", fi->input_file); - return -1; } data_writer_t *data_writer_create(sqfs_super_t *super, compressor_t *cmp, diff --git a/lib/sqfs/frag_reader.c b/lib/sqfs/frag_reader.c index 0d13bcd..6fa160b 100644 --- a/lib/sqfs/frag_reader.c +++ b/lib/sqfs/frag_reader.c @@ -42,16 +42,8 @@ static int precache_block(frag_reader_t *f, size_t i) return -1; } - ret = read_retry(f->fd, f->buffer, size); - if (ret < 0) { - perror("reading fragment"); + if (read_data("reading fragment", f->fd, f->buffer, size)) return -1; - } - - if ((size_t)ret < size) { - fputs("reading fragment: unexpected end of file\n", stderr); - return -1; - } if (compressed) { ret = f->cmp->do_block(f->cmp, f->buffer, size, @@ -62,11 +54,12 @@ static int precache_block(frag_reader_t *f, size_t i) return -1; } + size = ret; memmove(f->buffer, f->buffer + f->block_size, ret); } f->current_index = i; - f->used = ret; + f->used = size; return 0; } @@ -79,7 +72,6 @@ frag_reader_t *frag_reader_create(sqfs_super_t *super, int fd, uint64_t *locations = NULL; meta_reader_t *m = NULL; frag_reader_t *f = NULL; - ssize_t ret; count = super->fragment_entry_count; blockcount = count / (SQFS_META_BLOCK_SIZE / sizeof(tbl[0])); @@ -104,12 +96,10 @@ frag_reader_t *frag_reader_create(sqfs_super_t *super, int fd, if (lseek(fd, super->fragment_table_start, SEEK_SET) == (off_t)-1) goto fail_seek; - ret = read_retry(fd, locations, blockcount * sizeof(locations[0])); - if (ret < 0) - goto fail_rd; - - if ((size_t)ret < (blockcount * sizeof(locations[0]))) - goto fail_trunc; + if (read_data("reading fragment table", fd, locations, + blockcount * sizeof(locations[0]))) { + goto fail; + } for (i = 0; i < blockcount; ++i) locations[i] = le64toh(locations[i]); @@ -150,9 +140,6 @@ frag_reader_t *frag_reader_create(sqfs_super_t *super, int fd, fail_seek: perror("seek to fragment table"); goto fail; -fail_trunc: - fputs("reading fragment table: unexpected end of file\n", stderr); - goto fail; fail_rd: perror("reading fragment table"); goto fail; diff --git a/lib/sqfs/id_table_read.c b/lib/sqfs/id_table_read.c index 58b0cf9..e13f3b1 100644 --- a/lib/sqfs/id_table_read.c +++ b/lib/sqfs/id_table_read.c @@ -14,7 +14,6 @@ int id_table_read(id_table_t *tbl, int fd, sqfs_super_t *super, uint64_t blocks[32]; meta_reader_t *m; uint32_t *ptr; - ssize_t ret; if (tbl->ids != NULL) { free(tbl->ids); @@ -44,12 +43,10 @@ int id_table_read(id_table_t *tbl, int fd, sqfs_super_t *super, if (super->id_count % 2048) ++block_count; - ret = read_retry(fd, blocks, sizeof(blocks[0]) * block_count); - if (ret < 0) - goto fail_rd; - - if ((size_t)ret < sizeof(blocks[0]) * block_count) - goto fail_trunc; + if (read_data("reading ID table", fd, blocks, + sizeof(blocks[0]) * block_count)) { + return -1; + } for (i = 0; i < block_count; ++i) blocks[i] = le64toh(blocks[i]); @@ -83,12 +80,6 @@ int id_table_read(id_table_t *tbl, int fd, sqfs_super_t *super, fail_meta: meta_reader_destroy(m); return -1; -fail_trunc: - fputs("reading ID table: unexpected end of file\n", stderr); - return -1; -fail_rd: - perror("reading ID table"); - return -1; fail_seek: perror("seeking to ID table"); return -1; diff --git a/lib/sqfs/meta_reader.c b/lib/sqfs/meta_reader.c index e9b599f..c85d063 100644 --- a/lib/sqfs/meta_reader.c +++ b/lib/sqfs/meta_reader.c @@ -71,12 +71,8 @@ int meta_reader_seek(meta_reader_t *m, uint64_t block_start, size_t offset) return -1; } - ret = read_retry(m->fd, &header, 2); - if (ret < 0) - goto fail_rd; - - if ((size_t)ret < 2) - goto fail_trunc; + if (read_data("reading meta data header", m->fd, &header, 2)) + return -1; header = le16toh(header); compressed = (header & 0x8000) == 0; @@ -94,12 +90,8 @@ int meta_reader_seek(meta_reader_t *m, uint64_t block_start, size_t offset) memset(m->data, 0, sizeof(m->data)); - ret = read_retry(m->fd, m->data, size); - if (ret < 0) - goto fail_rd; - - if ((size_t)ret < size) - goto fail_trunc; + if (read_data("reading meta data block", m->fd, m->data, size)) + return -1; if (compressed) { ret = m->cmp->do_block(m->cmp, m->data, size, @@ -117,12 +109,6 @@ int meta_reader_seek(meta_reader_t *m, uint64_t block_start, size_t offset) } return 0; -fail_trunc: - fputs("reading meta data: unexpected end of file\n", stderr); - return -1; -fail_rd: - perror("reading meta data"); - return -1; } int meta_reader_read(meta_reader_t *m, void *data, size_t size) diff --git a/lib/sqfs/read_super.c b/lib/sqfs/read_super.c index 7b738ea..8c43fe0 100644 --- a/lib/sqfs/read_super.c +++ b/lib/sqfs/read_super.c @@ -11,23 +11,13 @@ int sqfs_super_read(sqfs_super_t *super, int fd) { size_t block_size = 0; sqfs_super_t temp; - ssize_t ret; int i; if (lseek(fd, 0, SEEK_SET) == (off_t)-1) goto fail_seek; - ret = read_retry(fd, &temp, sizeof(temp)); - - if (ret < 0) { - perror("reading super block"); + if (read_data("reading super block", fd, &temp, sizeof(temp))) return -1; - } - - if ((size_t)ret < sizeof(temp)) { - fputs("reading super block: unexpected end of file\n", stderr); - return -1; - } temp.magic = le32toh(temp.magic); temp.inode_count = le32toh(temp.inode_count); diff --git a/lib/tar/read_header.c b/lib/tar/read_header.c index 94c4a69..1769e0a 100644 --- a/lib/tar/read_header.c +++ b/lib/tar/read_header.c @@ -31,18 +31,12 @@ static int check_version(const tar_header_t *hdr) static char *record_to_memory(int fd, uint64_t size) { char *buffer = malloc(size + 1); - ssize_t ret; if (buffer == NULL) goto fail_errno; - ret = read_retry(fd, buffer, size); - if (ret == 0) - goto fail_eof; - if (ret < 0) - goto fail_errno; - if ((uint64_t)ret < size) - goto fail_eof; + if (read_data("reading tar record", fd, buffer, size)) + goto fail; if (skip_padding(fd, size)) goto fail; @@ -52,9 +46,6 @@ static char *record_to_memory(int fd, uint64_t size) fail_errno: perror("reading tar record"); goto fail; -fail_eof: - fputs("reading tar record: unexpected end of file\n", stderr); - goto fail; fail: free(buffer); return NULL; @@ -416,18 +407,13 @@ int read_header(int fd, tar_header_decoded_t *out) bool prev_was_zero = false; uint64_t pax_size; tar_header_t hdr; - int ret, version; + int version; memset(out, 0, sizeof(*out)); for (;;) { - ret = read_retry(fd, &hdr, sizeof(hdr)); - if (ret == 0) - goto out_eof; - if (ret < 0) - goto fail_errno; - if (ret < (int)sizeof(hdr)) - goto fail_eof; + if (read_data("reading tar header", fd, &hdr, sizeof(hdr))) + goto fail; if (is_zero_block(&hdr)) { if (prev_was_zero) @@ -499,12 +485,6 @@ int read_header(int fd, tar_header_decoded_t *out) out_eof: clear_header(out); return 1; -fail_errno: - perror("reading tar header"); - goto fail; -fail_eof: - fputs("reading tar header: unexpected end of file\n", stderr); - goto fail; fail_magic: fputs("input is not a ustar tar archive!\n", stderr); goto fail; diff --git a/lib/tar/read_sparse_map_old.c b/lib/tar/read_sparse_map_old.c index c0b2d0d..41d309a 100644 --- a/lib/tar/read_sparse_map_old.c +++ b/lib/tar/read_sparse_map_old.c @@ -6,7 +6,6 @@ sparse_map_t *read_gnu_old_sparse(int fd, tar_header_t *hdr) sparse_map_t *list = NULL, *end = NULL, *node; gnu_sparse_t sph; uint64_t off, sz; - ssize_t ret; int i; for (i = 0; i < 4; ++i) { @@ -41,11 +40,10 @@ sparse_map_t *read_gnu_old_sparse(int fd, tar_header_t *hdr) return list; do { - ret = read_retry(fd, &sph, sizeof(sph)); - if (ret < 0) - goto fail_errno; - if ((size_t)ret < sizeof(sph)) - goto fail_eof; + if (read_data("reading GNU sparse header", + fd, &sph, sizeof(sph))) { + goto fail; + } for (i = 0; i < 21; ++i) { if (!isdigit(sph.sparse[i].offset[0])) @@ -77,9 +75,6 @@ sparse_map_t *read_gnu_old_sparse(int fd, tar_header_t *hdr) } while (sph.isextended != 0); return list; -fail_eof: - fputs("parsing GNU sparse header: unexpected end of file", stderr); - goto fail; fail_errno: perror("parsing GNU sparse header"); goto fail; diff --git a/lib/tar/skip.c b/lib/tar/skip.c index 69208b2..28c02be 100644 --- a/lib/tar/skip.c +++ b/lib/tar/skip.c @@ -7,7 +7,6 @@ static int skip_bytes(int fd, uint64_t size) { unsigned char buffer[1024]; - ssize_t ret; size_t diff; while (size != 0) { @@ -15,17 +14,8 @@ static int skip_bytes(int fd, uint64_t size) if (diff > size) diff = size; - ret = read_retry(fd, buffer, diff); - - if (ret < 0) { - perror("reading tar record data"); - return -1; - } - - if ((size_t)ret < diff) { - fputs("unexpected end of file\n", stderr); + if (read_data("reading tar record padding", fd, buffer, diff)) return -1; - } size -= diff; } diff --git a/lib/util/read_data.c b/lib/util/read_data.c new file mode 100644 index 0000000..43f8da8 --- /dev/null +++ b/lib/util/read_data.c @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-3.0-or-later */ +#include +#include +#include + +#include "util.h" + +int read_data(const char *errstr, int fd, void *buffer, size_t size) +{ + ssize_t ret; + + while (size > 0) { + ret = read(fd, buffer, size); + if (ret < 0) { + if (errno == EINTR) + continue; + perror(errstr); + return -1; + } + if (ret == 0) { + fprintf(stderr, "%s: short read\n", errstr); + return -1; + } + + size -= ret; + buffer = (char *)buffer + ret; + } + + return 0; +} diff --git a/lib/util/read_retry.c b/lib/util/read_retry.c deleted file mode 100644 index eb113c4..0000000 --- a/lib/util/read_retry.c +++ /dev/null @@ -1,27 +0,0 @@ -/* SPDX-License-Identifier: GPL-3.0-or-later */ -#include -#include - -#include "util.h" - -ssize_t read_retry(int fd, void *buffer, size_t size) -{ - ssize_t ret, total = 0; - - while (size > 0) { - ret = read(fd, buffer, size); - if (ret < 0) { - if (errno == EINTR) - continue; - return -1; - } - if (ret == 0) - break; - - total += ret; - size -= ret; - buffer = (char *)buffer + ret; - } - - return total; -} -- cgit v1.2.3