From owner-p4-projects@FreeBSD.ORG Sat May 15 05:13:17 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 9B7DA1065679; Sat, 15 May 2010 05:13:17 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6029D1065676 for ; Sat, 15 May 2010 05:13:17 +0000 (UTC) (envelope-from gcooper@FreeBSD.org) Received: from repoman.freebsd.org (unknown [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 4CAAC8FC13 for ; Sat, 15 May 2010 05:13:17 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id o4F5DG2h037160 for ; Sat, 15 May 2010 05:13:16 GMT (envelope-from gcooper@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o4F5DGRt037158 for perforce@freebsd.org; Sat, 15 May 2010 05:13:16 GMT (envelope-from gcooper@FreeBSD.org) Date: Sat, 15 May 2010 05:13:16 GMT Message-Id: <201005150513.o4F5DGRt037158@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to gcooper@FreeBSD.org using -f From: Garrett Cooper To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 178288 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 May 2010 05:13:17 -0000 http://p4web.freebsd.org/@@178288?ac=10 Change 178288 by gcooper@starr-bastion on 2010/05/15 05:12:21 1. Rename unpack to unpack_to_disk for consistency with archive(3). 2. Shuffle around unpack_to_disk so that it's in proper sorted order. Affected files ... .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/file.c#12 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/pkg.h#6 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/add/perform.c#9 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/info/perform.c#4 edit Differences ... ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/file.c#12 (text+ko) ==== @@ -384,49 +384,43 @@ } -/* - * Unpack a single file from a tar-file to a file descriptor; this is written - * like so as an optimization to abbreviate the extract to *open step, as well - * as to reduce the number of required steps needed when unpacking a tarball on - * disk, as the previous method employed with tar(1) used -q // --fast-read . +/* + * Unpack a tar file, or a subset of the contents. * - * Return NULL on failure, and non-NULL on success + * Return 0 on success, 1 on failure * - * XXX (gcooper): this is currently implemented with FILE* / fopen(3) due to - * legacy issues that need to be sorted out over the next couple of weeks for - * 1) locking to function properly, and to gain the potential performance boost - * by using mmap(2), and read(2) (ugh). - * - * But first things first, we need a working solution with minimal changes; - * then we move on from there. - * - * [The return code info will eventually be...] - * - * Return -1 on failure, a value greater than 0 on success. + * NOTE: the exit code is 0 / 1 so that this can be fed directly into exit + * when doing piped tar commands for copying hierarchies *hint*, *hint* -- this + * use may be short-lived though, so don't depend on the return value, mmk? */ -FILE* -unpack_to_fd(const char *pkg, const char *file) +int +unpack_to_disk(const char *pkg, const char *file_expr) { struct archive *archive; struct archive_entry *archive_entry; - Boolean found_match = FALSE; - + Boolean extract_whole_archive = FALSE; const char *entry_pathname = NULL; const char *error = NULL; const char *pkg_name_humanized; + int archive_fd = -1, r; - FILE *fd = NULL; - /* int fd = -1; */ - int archive_fd = -1, r; + if (file_expr == NULL || strcmp("*", file_expr) == 0) + extract_whole_archive = TRUE; if (pkg == NULL || strcmp(pkg, "-") == 0) pkg_name_humanized = "(stdin)"; else pkg_name_humanized = pkg; - if (Verbose) - printf("%s: will extract %s from %s\n", - __func__, file, pkg_name_humanized); + if (Verbose) { + if (extract_whole_archive) + printf("%s: %s - will extract whole archive\n", + pkg_name_humanized, __func__); + else + printf("%s: %s - will extract files that match " + "expression: %s\n", + pkg_name_humanized, __func__, file_expr); + } if ((archive = archive_read_new()) != NULL) { @@ -451,26 +445,23 @@ /* archive(3) failed to open the file descriptor. */ else if (archive_read_open_fd(archive, archive_fd, ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + error = archive_error_string(archive); warnx("%s: unable to open the package from %s: %s", __func__, pkg_name_humanized, error); + } else - while (error == NULL && found_match == FALSE && + while (error == NULL && (r = archive_read_next_header(archive, &archive_entry)) == ARCHIVE_OK) { entry_pathname = archive_entry_pathname(archive_entry); - if (strncmp(file, entry_pathname, PATH_MAX) == 0) { - - /* - * Regardless of whether or not extract passes, - * we found our target file so let's exit - * quickly because the underlying issue is most - * likely unrecoverable. - */ - found_match = TRUE; + /* Let's extract the whole archive, or just a file. */ + if (extract_whole_archive == TRUE || + (fnmatch(file_expr, entry_pathname, + FNM_PATHNAME)) == 0) { r = archive_read_extract(archive, archive_entry, EXTRACT_ARCHIVE_FLAGS); @@ -478,7 +469,6 @@ if (Verbose) printf("X - %s\n", entry_pathname); - fd = fopen(entry_pathname, "r"); } else { error = archive_error_string(archive); warnx("%s: extraction for %s failed: " @@ -495,8 +485,8 @@ if (errno != 0) error = strerror(errno); if (error != NULL) - warnx("%s: unable to read the file - %s - from package: %s: " - "%s", __func__, file, pkg_name_humanized, error); + warnx("%s: unpacking package - %s - failed: %s", + __func__, pkg_name_humanized, error); if (archive != NULL) archive_read_finish(archive); @@ -504,47 +494,53 @@ if (0 <= archive_fd) close(archive_fd); - return fd; + return (error == NULL ? 0 : 1); } -/* - * Unpack a tar file, or a subset of the contents. +/* + * Unpack a single file from a tar-file to a file descriptor; this is written + * like so as an optimization to abbreviate the extract to *open step, as well + * as to reduce the number of required steps needed when unpacking a tarball on + * disk, as the previous method employed with tar(1) used -q // --fast-read . + * + * Return NULL on failure, and non-NULL on success + * + * XXX (gcooper): this is currently implemented with FILE* / fopen(3) due to + * legacy issues that need to be sorted out over the next couple of weeks for + * 1) locking to function properly, and to gain the potential performance boost + * by using mmap(2), and read(2) (ugh). + * + * But first things first, we need a working solution with minimal changes; + * then we move on from there. * - * Return 0 on success, 1 on failure + * [The return code info will eventually be...] * - * NOTE: the exit code is 0 / 1 so that this can be fed directly into exit - * when doing piped tar commands for copying hierarchies *hint*, *hint* -- this - * use may be short-lived though, so don't depend on the return value, mmk? + * Return -1 on failure, a value greater than 0 on success. */ -int -unpack(const char *pkg, const char *file_expr) +FILE* +unpack_to_fd(const char *pkg, const char *file) { struct archive *archive; struct archive_entry *archive_entry; - Boolean extract_whole_archive = FALSE; + Boolean found_match = FALSE; + const char *entry_pathname = NULL; const char *error = NULL; const char *pkg_name_humanized; + + FILE *fd = NULL; + /* int fd = -1; */ int archive_fd = -1, r; - if (file_expr == NULL || strcmp("*", file_expr) == 0) - extract_whole_archive = TRUE; - if (pkg == NULL || strcmp(pkg, "-") == 0) pkg_name_humanized = "(stdin)"; else pkg_name_humanized = pkg; - if (Verbose) { - if (extract_whole_archive) - printf("%s: %s - will extract whole archive\n", - pkg_name_humanized, __func__); - else - printf("%s: %s - will extract files that match " - "expression: %s\n", - pkg_name_humanized, __func__, file_expr); - } + if (Verbose) + printf("%s: will extract %s from %s\n", + __func__, file, pkg_name_humanized); if ((archive = archive_read_new()) != NULL) { @@ -569,23 +565,26 @@ /* archive(3) failed to open the file descriptor. */ else if (archive_read_open_fd(archive, archive_fd, ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { - error = archive_error_string(archive); warnx("%s: unable to open the package from %s: %s", __func__, pkg_name_humanized, error); - } else - while (error == NULL && + while (error == NULL && found_match == FALSE && (r = archive_read_next_header(archive, &archive_entry)) == ARCHIVE_OK) { entry_pathname = archive_entry_pathname(archive_entry); - /* Let's extract the whole archive, or just a file. */ - if (extract_whole_archive == TRUE || - (fnmatch(file_expr, entry_pathname, - FNM_PATHNAME)) == 0) { + if (strncmp(file, entry_pathname, PATH_MAX) == 0) { + + /* + * Regardless of whether or not extract passes, + * we found our target file so let's exit + * quickly because the underlying issue is most + * likely unrecoverable. + */ + found_match = TRUE; r = archive_read_extract(archive, archive_entry, EXTRACT_ARCHIVE_FLAGS); @@ -593,6 +592,7 @@ if (Verbose) printf("X - %s\n", entry_pathname); + fd = fopen(entry_pathname, "r"); } else { error = archive_error_string(archive); warnx("%s: extraction for %s failed: " @@ -609,8 +609,8 @@ if (errno != 0) error = strerror(errno); if (error != NULL) - warnx("%s: unpacking package - %s - failed: %s", - __func__, pkg_name_humanized, error); + warnx("%s: unable to read the file - %s - from package: %s: " + "%s", __func__, file, pkg_name_humanized, error); if (archive != NULL) archive_read_finish(archive); @@ -618,7 +618,7 @@ if (0 <= archive_fd) close(archive_fd); - return (error == NULL ? 0 : 1); + return fd; } ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/pkg.h#6 (text+ko) ==== @@ -188,8 +188,8 @@ void move_file(const char *, const char *, const char *); void copy_hierarchy(const char *, const char *, Boolean); int delete_hierarchy(const char *, Boolean, Boolean); -int unpack(const char *, const char *); char* unpack_to_buffer(const char *, const char *); +int unpack_to_disk(const char *, const char *); FILE* unpack_to_fd(const char *, const char *); void format_cmd(char *, int, const char *, const char *, const char *); ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/add/perform.c#9 (text+ko) ==== @@ -139,7 +139,7 @@ if (!getenv("_TOP")) setenv("_TOP", where_to, 1); if (extract_whole_archive_from_stdin == TRUE) { - if (unpack(NULL, NULL) == 0) + if (unpack_to_disk(NULL, NULL) == 0) cfile = fopen(CONTENTS_FNAME, "r"); else { warnx("unable to extract table of contents file from '%s' " @@ -204,7 +204,7 @@ /* Finally unpack the whole mess. If extract is null we already + did so so don't bother doing it again. */ - if (extract && unpack(pkg, NULL)) { + if (extract && unpack_to_disk(pkg, NULL)) { warnx("unable to extract '%s'!", pkg); goto bomb; } ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/info/perform.c#4 (text+ko) ==== @@ -137,7 +137,7 @@ goto bail; } make_playpen(PlayPen, sb.st_size / 2); - if (unpack(fname, "+*")) { + if (unpack_to_disk(fname, "+*")) { warnx("error during unpacking, no info for '%s' available", pkg); code = 1; goto bail;