PERFORCE change 178288 for review

Garrett Cooper gcooper at FreeBSD.org
Sat May 15 05:13:17 UTC 2010


http://p4web.freebsd.org/@@178288?ac=10

Change 178288 by gcooper at 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;


More information about the p4-projects mailing list