Date: Sun, 12 Apr 2009 05:47:23 +0000 (UTC) From: Tim Kientzle <kientzle@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r190961 - in head/lib/libarchive: . test Message-ID: <200904120547.n3C5lNVm012363@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kientzle Date: Sun Apr 12 05:47:23 2009 New Revision: 190961 URL: http://svn.freebsd.org/changeset/base/190961 Log: Merge from libarchive.googlecode.com r791, r879, r884, r948: Various fixes to read_support_compression_program. In particular, failure of the external program is detected a lot earlier, which gives much more reasonable error handling. Modified: head/lib/libarchive/archive_read_support_compression_program.c head/lib/libarchive/test/test_read_compress_program.c Modified: head/lib/libarchive/archive_read_support_compression_program.c ============================================================================== --- head/lib/libarchive/archive_read_support_compression_program.c Sun Apr 12 05:38:35 2009 (r190960) +++ head/lib/libarchive/archive_read_support_compression_program.c Sun Apr 12 05:47:23 2009 (r190961) @@ -38,6 +38,9 @@ __FBSDID("$FreeBSD$"); #ifdef HAVE_LIMITS_H # include <limits.h> #endif +#ifdef HAVE_SIGNAL_H +# include <signal.h> +#endif #ifdef HAVE_STDLIB_H # include <stdlib.h> #endif @@ -119,6 +122,8 @@ static int program_bidder_free(struct ar struct program_filter { char *description; pid_t child; + int exit_status; + int waitpid_return; int child_stdin, child_stdout; char *out_buf; @@ -211,6 +216,73 @@ program_bidder_bid(struct archive_read_f } /* + * Shut down the child, return ARCHIVE_OK if it exited normally. + * + * Note that the return value is sticky; if we're called again, + * we won't reap the child again, but we will return the same status + * (including error message if the child came to a bad end). + */ +static int +child_stop(struct archive_read_filter *self, struct program_filter *state) +{ + /* Close our side of the I/O with the child. */ + if (state->child_stdin != -1) { + close(state->child_stdin); + state->child_stdin = -1; + } + if (state->child_stdout != -1) { + close(state->child_stdout); + state->child_stdout = -1; + } + + if (state->child != 0) { + /* Reap the child. */ + do { + state->waitpid_return + = waitpid(state->child, &state->exit_status, 0); + } while (state->waitpid_return == -1 && errno == EINTR); + state->child = 0; + } + + if (state->waitpid_return < 0) { + /* waitpid() failed? This is ugly. */ + archive_set_error(&self->archive->archive, ARCHIVE_ERRNO_MISC, + "Child process exited badly"); + return (ARCHIVE_WARN); + } + + if (WIFSIGNALED(state->exit_status)) { +#ifdef SIGPIPE + /* If the child died because we stopped reading before + * it was done, that's okay. Some archive formats + * have padding at the end that we routinely ignore. */ + /* The alternative to this would be to add a step + * before close(child_stdout) above to read from the + * child until the child has no more to write. */ + if (WTERMSIG(state->exit_status) == SIGPIPE) + return (ARCHIVE_OK); +#endif + archive_set_error(&self->archive->archive, ARCHIVE_ERRNO_MISC, + "Child process exited with signal %d", + WTERMSIG(state->exit_status)); + return (ARCHIVE_WARN); + } + + if (WIFEXITED(state->exit_status)) { + if (WEXITSTATUS(state->exit_status) == 0) + return (ARCHIVE_OK); + + archive_set_error(&self->archive->archive, + ARCHIVE_ERRNO_MISC, + "Child process exited with status %d", + WEXITSTATUS(state->exit_status)); + return (ARCHIVE_WARN); + } + + return (ARCHIVE_WARN); +} + +/* * Use select() to decide whether the child is ready for read or write. */ static ssize_t @@ -229,11 +301,10 @@ child_read(struct archive_read_filter *s if (ret > 0) return (ret); - if (ret == 0 || (ret == -1 && errno == EPIPE)) { - close(state->child_stdout); - state->child_stdout = -1; - return (0); - } + if (ret == 0 || (ret == -1 && errno == EPIPE)) + /* Child has closed its output; reap the child + * and return the status. */ + return (child_stop(self, state)); if (ret == -1 && errno != EAGAIN) return (-1); @@ -352,8 +423,11 @@ program_filter_read(struct archive_read_ while (state->child_stdout != -1 && total < state->out_buf_len) { bytes = child_read(self, p, state->out_buf_len - total); if (bytes < 0) - return (bytes); + /* No recovery is possible if we can no longer + * read from the child. */ + return (ARCHIVE_FATAL); if (bytes == 0) + /* We got EOF from the child. */ break; total += bytes; p += bytes; @@ -367,24 +441,17 @@ static int program_filter_close(struct archive_read_filter *self) { struct program_filter *state; - int status; + int e; state = (struct program_filter *)self->data; - - /* Shut down the child. */ - if (state->child_stdin != -1) - close(state->child_stdin); - if (state->child_stdout != -1) - close(state->child_stdout); - while (waitpid(state->child, &status, 0) == -1 && errno == EINTR) - continue; + e = child_stop(self, state); /* Release our private data. */ free(state->out_buf); free(state->description); free(state); - return (ARCHIVE_OK); + return (e); } #endif /* !defined(HAVE_PIPE) || !defined(HAVE_VFORK) || !defined(HAVE_FCNTL) */ Modified: head/lib/libarchive/test/test_read_compress_program.c ============================================================================== --- head/lib/libarchive/test/test_read_compress_program.c Sun Apr 12 05:38:35 2009 (r190960) +++ head/lib/libarchive/test/test_read_compress_program.c Sun Apr 12 05:47:23 2009 (r190961) @@ -35,14 +35,34 @@ static unsigned char archive[] = { DEFINE_TEST(test_read_compress_program) { int r; - -#if ARCHIVE_VERSION_NUMBER < 1009000 - skipping("archive_read_support_compression_program()"); -#else struct archive_entry *ae; struct archive *a; const char *extprog; + /* + * First, test handling when a non-existent compression + * program is requested. + */ + assert((a = archive_read_new()) != NULL); + r = archive_read_support_compression_program(a, "nonexistent"); + if (r == ARCHIVE_FATAL) { + skipping("archive_read_support_compression_program() " + "unsupported on this platform"); + return; + } + assertEqualIntA(a, ARCHIVE_OK, r); + assertEqualIntA(a, ARCHIVE_OK, + archive_read_support_format_all(a)); + assertEqualIntA(a, ARCHIVE_OK, + archive_read_open_memory(a, archive, sizeof(archive))); + assertEqualIntA(a, ARCHIVE_FATAL, + archive_read_next_header(a, &ae)); + assertEqualIntA(a, ARCHIVE_WARN, archive_read_close(a)); + assertEqualInt(ARCHIVE_OK, archive_read_finish(a)); + + /* + * If we have "gunzip", try using that. + */ if ((extprog = external_gzip_program(1)) == NULL) { skipping("There is no gzip uncompression " "program in this platform"); @@ -51,28 +71,18 @@ DEFINE_TEST(test_read_compress_program) assert((a = archive_read_new()) != NULL); assertEqualIntA(a, ARCHIVE_OK, archive_read_support_compression_none(a)); - r = archive_read_support_compression_program(a, extprog); - if (r == ARCHIVE_FATAL) { - skipping("archive_read_support_compression_program() " - "unsupported on this platform"); - return; - } - assertEqualIntA(a, ARCHIVE_OK, r); + assertEqualIntA(a, ARCHIVE_OK, + archive_read_support_compression_program(a, extprog)); assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a)); assertEqualIntA(a, ARCHIVE_OK, archive_read_open_memory(a, archive, sizeof(archive))); assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae)); - assert(archive_compression(a) == ARCHIVE_COMPRESSION_PROGRAM); - assert(archive_format(a) == ARCHIVE_FORMAT_TAR_USTAR); - assert(0 == archive_read_close(a)); -#if ARCHIVE_VERSION_NUMBER < 2000000 - archive_read_finish(a); -#else - assert(0 == archive_read_finish(a)); -#endif -#endif + assertEqualInt(archive_compression(a), ARCHIVE_COMPRESSION_PROGRAM); + assertEqualInt(archive_format(a), ARCHIVE_FORMAT_TAR_USTAR); + assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a)); + assertEqualInt(ARCHIVE_OK, archive_read_finish(a)); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200904120547.n3C5lNVm012363>