From bfd876dbf151df164b4d87de20aec39b24f205f9 Mon Sep 17 00:00:00 2001 From: David Oberhollenzer Date: Tue, 16 Jul 2019 19:29:27 +0200 Subject: cleanup: move error handling into write_retry If write_retry fails to write everything, it is *always* an error. This commit renames write_retry to write_data and moves error handling into the function, making a lot of error handling code redundant. Signed-off-by: David Oberhollenzer --- include/util.h | 6 ++++-- lib/Makemodule.am | 2 +- lib/comp/compressor.c | 14 +++----------- lib/sqfs/data_reader.c | 26 ++++++++------------------ lib/sqfs/data_writer.c | 14 +++----------- lib/sqfs/meta_writer.c | 20 +++----------------- lib/sqfs/super.c | 13 +------------ lib/sqfs/table.c | 11 +---------- lib/sqfs/write_xattr.c | 23 ++++++----------------- lib/tar/write_header.c | 25 +++---------------------- lib/util/padd_file.c | 15 ++++----------- lib/util/write_data.c | 30 ++++++++++++++++++++++++++++++ lib/util/write_retry.c | 27 --------------------------- tar/sqfs2tar.c | 16 ++-------------- 14 files changed, 69 insertions(+), 173 deletions(-) create mode 100644 lib/util/write_data.c delete mode 100644 lib/util/write_retry.c diff --git a/include/util.h b/include/util.h index 997cbf5..aff313f 100644 --- a/include/util.h +++ b/include/util.h @@ -23,9 +23,11 @@ int canonicalize_name(char *filename); /* A wrapper around the write() system call. It retries the write if it is - interrupted by a signal or only part of the data was written. + interrupted by a signal or only part of the data was written. Returns 0 + on success. Writes to stderr on failure using 'errstr' as a perror style + error prefix. */ -ssize_t write_retry(int fd, const void *data, size_t size); +int write_data(const char *errstr, int fd, const void *data, size_t size); /* A wrapper around the read() system call. It retries the read if it is diff --git a/lib/Makemodule.am b/lib/Makemodule.am index 865d99a..710db78 100644 --- a/lib/Makemodule.am +++ b/lib/Makemodule.am @@ -36,7 +36,7 @@ libsquashfs_a_SOURCES += lib/sqfs/data_writer.c lib/sqfs/write_xattr.c 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_retry.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/print_version.c lib/util/mkdir_p.c libutil_a_SOURCES += lib/util/str_table.c include/str_table.h diff --git a/lib/comp/compressor.c b/lib/comp/compressor.c index cc04aa6..96bc3b1 100644 --- a/lib/comp/compressor.c +++ b/lib/comp/compressor.c @@ -59,24 +59,16 @@ static const char *names[] = { int generic_write_options(int fd, const void *data, size_t size) { uint8_t buffer[size + 2]; - ssize_t ret; *((uint16_t *)buffer) = htole16(0x8000 | size); memcpy(buffer + 2, data, size); - ret = write_retry(fd, buffer, sizeof(buffer)); - - if (ret < 0) { - perror("writing compressor options"); - return -1; - } - - if ((size_t)ret < sizeof(buffer)) { - fputs("writing compressor options: truncated write\n", stderr); + if (write_data("writing compressor options", + fd, buffer, sizeof(buffer))) { return -1; } - return ret; + return sizeof(buffer); } int generic_read_options(int fd, void *data, size_t size) diff --git a/lib/sqfs/data_reader.c b/lib/sqfs/data_reader.c index ec85412..7f857b2 100644 --- a/lib/sqfs/data_reader.c +++ b/lib/sqfs/data_reader.c @@ -133,12 +133,10 @@ int data_reader_dump_file(data_reader_t *data, file_info_t *fi, int outfd, ptr = data->buffer; } - ret = write_retry(outfd, ptr, unpackedsz); - if (ret < 0) - goto fail_wr; - - if ((size_t)ret < bs) - goto fail_wr_trunc; + if (write_data("writing uncompressed block", + outfd, ptr, unpackedsz)) { + return -1; + } } } @@ -152,12 +150,10 @@ int data_reader_dump_file(data_reader_t *data, file_info_t *fi, int outfd, return -1; } - ret = write_retry(outfd, data->buffer, fragsz); - if (ret < 0) - goto fail_wr; - - if ((size_t)ret < fragsz) - goto fail_wr_trunc; + if (write_data("writing uncompressed fragment", + outfd, data->buffer, fragsz)) { + return -1; + } } return 0; @@ -167,12 +163,6 @@ fail_sparse: fail_seek: perror("seek on squashfs"); return -1; -fail_wr: - perror("writing uncompressed block"); - return -1; -fail_wr_trunc: - fputs("writing uncompressed block: truncated write\n", stderr); - return -1; fail_rd: perror("reading from squashfs"); return -1; diff --git a/lib/sqfs/data_writer.c b/lib/sqfs/data_writer.c index cbe2351..eebd2a4 100644 --- a/lib/sqfs/data_writer.c +++ b/lib/sqfs/data_writer.c @@ -41,24 +41,16 @@ static int write_compressed(data_writer_t *data, const void *in, size_t size, if (ret > 0 && (size_t)ret < size) { size = ret; - ret = write_retry(data->outfd, data->scratch, size); + in = data->scratch; *outsize = size; } else { - ret = write_retry(data->outfd, in, size); *outsize = size | (1 << 24); } - if (ret < 0) { - perror("writing to output file"); + if (write_data("writing data block", data->outfd, in, size)) return -1; - } - - if ((size_t)ret < size) { - fputs("write to output file truncated\n", stderr); - return -1; - } - data->super->bytes_used += ret; + data->super->bytes_used += size; return 0; } diff --git a/lib/sqfs/meta_writer.c b/lib/sqfs/meta_writer.c index ef869e7..d55af3d 100644 --- a/lib/sqfs/meta_writer.c +++ b/lib/sqfs/meta_writer.c @@ -38,24 +38,10 @@ struct meta_writer_t { static int write_block(int fd, meta_block_t *outblk) { - size_t count; - ssize_t ret; - - count = le16toh(((uint16_t *)outblk->data)[0]) & 0x7FFF; - - ret = write_retry(fd, outblk->data, count + 2); - - if (ret < 0) { - perror("writing meta data block"); - return -1; - } - - if ((size_t)ret < count) { - fputs("meta data written to file was truncated\n", stderr); - return -1; - } + size_t count = le16toh(((uint16_t *)outblk->data)[0]) & 0x7FFF; - return 0; + return write_data("writing meta data block", fd, + outblk->data, count + 2); } meta_writer_t *meta_writer_create(int fd, compressor_t *cmp, bool keep_in_mem) diff --git a/lib/sqfs/super.c b/lib/sqfs/super.c index d3b79c6..59eb366 100644 --- a/lib/sqfs/super.c +++ b/lib/sqfs/super.c @@ -47,7 +47,6 @@ int sqfs_super_init(sqfs_super_t *super, size_t block_size, uint32_t mtime, int sqfs_super_write(sqfs_super_t *super, int fd) { sqfs_super_t copy; - ssize_t ret; copy.magic = htole32(super->magic); copy.inode_count = htole32(super->inode_count); @@ -72,18 +71,8 @@ int sqfs_super_write(sqfs_super_t *super, int fd) if (lseek(fd, 0, SEEK_SET) == (off_t)-1) goto fail_seek; - ret = write_retry(fd, ©, sizeof(copy)); - - if (ret < 0) { - perror("squashfs writing super block"); + if (write_data("writing super block", fd, ©, sizeof(copy))) return -1; - } - - if ((size_t)ret < sizeof(copy)) { - fputs("squashfs writing super block: truncated write\n", - stderr); - return -1; - } if (lseek(fd, 0, SEEK_END) == (off_t)-1) goto fail_seek; diff --git a/lib/sqfs/table.c b/lib/sqfs/table.c index fb6bddc..81e7691 100644 --- a/lib/sqfs/table.c +++ b/lib/sqfs/table.c @@ -15,7 +15,6 @@ int sqfs_write_table(int outfd, sqfs_super_t *super, const void *data, size_t i, blkidx = 0, tblsize; meta_writer_t *m; uint32_t offset; - ssize_t ret; /* Write actual data. Whenever we cross a block boundary, remember the block start offset */ @@ -49,16 +48,8 @@ int sqfs_write_table(int outfd, sqfs_super_t *super, const void *data, *startblock = super->bytes_used; tblsize = sizeof(blocks[0]) * blkidx; - ret = write_retry(outfd, blocks, tblsize); - if (ret < 0) { - perror("writing index table"); + if (write_data("writing table index", outfd, blocks, tblsize)) return -1; - } - - if ((size_t)ret < tblsize) { - fputs("index table truncated\n", stderr); - return -1; - } super->bytes_used += tblsize; return 0; diff --git a/lib/sqfs/write_xattr.c b/lib/sqfs/write_xattr.c index 0e11757..31fdf50 100644 --- a/lib/sqfs/write_xattr.c +++ b/lib/sqfs/write_xattr.c @@ -105,7 +105,6 @@ int write_xattr(int outfd, fstree_t *fs, sqfs_super_t *super, meta_writer_t *mw; tree_xattr_t *it; uint32_t offset; - ssize_t ret; if (fs->xattr == NULL) return 0; @@ -179,17 +178,13 @@ int write_xattr(int outfd, fstree_t *fs, sqfs_super_t *super, idtbl.xattr_ids = htole32(count); idtbl.unused = 0; - ret = write_retry(outfd, &idtbl, sizeof(idtbl)); - if (ret < 0) - goto fail_wr; - if ((size_t)ret < sizeof(idtbl)) - goto fail_trunc; + if (write_data("writing xattr ID table", outfd, &idtbl, sizeof(idtbl))) + goto fail_tbl; - ret = write_retry(outfd, tbl, sizeof(tbl[0]) * blocks); - if (ret < 0) - goto fail_wr; - if ((size_t)ret < sizeof(tbl[0]) * blocks) - goto fail_trunc; + if (write_data("writing xattr ID table", + outfd, tbl, sizeof(tbl[0]) * blocks)) { + goto fail_tbl; + } super->xattr_id_table_start = super->bytes_used; super->bytes_used += sizeof(idtbl) + sizeof(tbl[0]) * blocks; @@ -197,12 +192,6 @@ int write_xattr(int outfd, fstree_t *fs, sqfs_super_t *super, free(tbl); meta_writer_destroy(mw); return 0; -fail_wr: - perror("writing xattr ID table"); - goto fail_tbl; -fail_trunc: - fputs("writing xattr ID table: truncated write\n", stderr); - goto fail_tbl; fail_tbl: free(tbl); fail_mw: diff --git a/lib/tar/write_header.c b/lib/tar/write_header.c index 70acb1a..caddc97 100644 --- a/lib/tar/write_header.c +++ b/lib/tar/write_header.c @@ -52,7 +52,6 @@ static int write_header(int fd, const struct stat *sb, const char *name, int maj = 0, min = 0; uint64_t size = 0; tar_header_t hdr; - ssize_t ret; if (S_ISCHR(sb->st_mode) || S_ISBLK(sb->st_mode)) { maj = major(sb->st_rdev); @@ -82,18 +81,7 @@ static int write_header(int fd, const struct stat *sb, const char *name, update_checksum(&hdr); - ret = write_retry(fd, &hdr, sizeof(hdr)); - if (ret < 0) { - perror("writing header record"); - return -1; - } - - if ((size_t)ret < sizeof(hdr)) { - fputs("writing header record: truncated write\n", stderr); - return -1; - } - - return 0; + return write_data("writing tar header record", fd, &hdr, sizeof(hdr)); } static int write_gnu_header(int fd, const struct stat *orig, @@ -101,7 +89,6 @@ static int write_gnu_header(int fd, const struct stat *orig, int type, const char *name) { struct stat sb; - ssize_t ret; sb = *orig; sb.st_mode = S_IFREG | 0644; @@ -110,14 +97,8 @@ static int write_gnu_header(int fd, const struct stat *orig, if (write_header(fd, &sb, name, NULL, type)) return -1; - ret = write_retry(fd, payload, payload_len); - if (ret < 0) { - perror("writing GNU extension header"); - return -1; - } - - if ((size_t)ret < payload_len) { - fputs("writing GNU extension header: truncated write\n", stderr); + if (write_data("writing GNU extension header", + fd, payload, payload_len)) { return -1; } diff --git a/lib/util/padd_file.c b/lib/util/padd_file.c index 8598b8b..2f1ea9a 100644 --- a/lib/util/padd_file.c +++ b/lib/util/padd_file.c @@ -9,7 +9,6 @@ int padd_file(int outfd, uint64_t size, size_t blocksize) size_t padd_sz = size % blocksize; int status = -1; uint8_t *buffer; - ssize_t ret; if (padd_sz == 0) return 0; @@ -20,21 +19,15 @@ int padd_file(int outfd, uint64_t size, size_t blocksize) if (buffer == NULL) goto fail_errno; - ret = write_retry(outfd, buffer, padd_sz); - - if (ret < 0) - goto fail_errno; - - if ((size_t)ret < padd_sz) - goto fail_trunc; + if (write_data("padding output file to block size", + outfd, buffer, padd_sz)) { + goto out; + } status = 0; out: free(buffer); return status; -fail_trunc: - fputs("padding output to block size: truncated write\n", stderr); - goto out; fail_errno: perror("padding output file to block size"); goto out; diff --git a/lib/util/write_data.c b/lib/util/write_data.c new file mode 100644 index 0000000..82f3aca --- /dev/null +++ b/lib/util/write_data.c @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-3.0-or-later */ +#include +#include +#include + +#include "util.h" + +int write_data(const char *errstr, int fd, const void *data, size_t size) +{ + ssize_t ret; + + while (size > 0) { + ret = write(fd, data, size); + if (ret == 0) { + fprintf(stderr, "%s: write truncated\n", errstr); + return -1; + } + if (ret < 0) { + if (errno == EINTR) + continue; + perror(errstr); + return -1; + } + + data = (const char *)data + ret; + size -= ret; + } + + return 0; +} diff --git a/lib/util/write_retry.c b/lib/util/write_retry.c deleted file mode 100644 index 0ef856c..0000000 --- a/lib/util/write_retry.c +++ /dev/null @@ -1,27 +0,0 @@ -/* SPDX-License-Identifier: GPL-3.0-or-later */ -#include -#include - -#include "util.h" - -ssize_t write_retry(int fd, const void *data, size_t size) -{ - ssize_t ret, total = 0; - - while (size > 0) { - ret = write(fd, data, size); - if (ret == 0) - break; - if (ret < 0) { - if (errno == EINTR) - continue; - return -1; - } - - data = (const char *)data + ret; - size -= ret; - total += ret; - } - - return total; -} diff --git a/tar/sqfs2tar.c b/tar/sqfs2tar.c index 110c8c4..15c0363 100644 --- a/tar/sqfs2tar.c +++ b/tar/sqfs2tar.c @@ -161,23 +161,11 @@ out_exit: static int terminate_archive(void) { char buffer[1024]; - ssize_t ret; memset(buffer, '\0', sizeof(buffer)); - ret = write_retry(STDOUT_FILENO, buffer, sizeof(buffer)); - - if (ret < 0) { - perror("adding archive terminator"); - return -1; - } - - if ((size_t)ret < sizeof(buffer)) { - fputs("adding archive terminator: truncated write\n", stderr); - return -1; - } - - return 0; + return write_data("adding archive terminator", STDOUT_FILENO, + buffer, sizeof(buffer)); } static int write_tree_dfs(fstree_t *fs, tree_node_t *n, data_reader_t *data) -- cgit v1.2.3