Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 1 Oct 2014 18:59:57 +0000 (UTC)
From:      Devin Teske <dteske@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r272379 - in head/usr.sbin/bsdinstall: distextract distfetch
Message-ID:  <201410011859.s91Ixvqa047151@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: dteske
Date: Wed Oct  1 18:59:57 2014
New Revision: 272379
URL: https://svnweb.freebsd.org/changeset/base/272379

Log:
  Optimize program flow for execution speed. Also fix some more style(9) nits
  while here:
  + Fix an issue when extracting small archives where dialog_mixedgauge was
    not rendering; leaving the user wondering if anything happened.
  + Add #ifdef's to assuage compilation against older libarchive
    NB: Minimize diff between branches; make merging easier.
  + Add missing calls to end_dialog(3)
  + Change string processing from strtok(3) to strcspn(3) (O(1) optimization)
  + Use EXIT_SUCCESS and EXIT_FAILURE instead of 0/1
  + Optimize getenv(3) use, using stored results instead of calling repeatedly
    NB: Fixes copy/paste error wherein we display getenv(BSDINSTALL_DISTDIR) in
        an error msgbox when chdir(2) to getenv(BSDINSTALL_CHROOT) fails
        (wrong variable displayed in msgbox).
  + Use strtol(3) instead of [deprecated] atoi(3)
  + Add additional error checking (e.g., check return of archive_read_new(3))
  + Assign DECONST strings to static variables
  + Fix typo in distextract.c error message (s/Could could/Could not/)
  + Add comments and make a minor whitespace adjustment
  
  Reviewed by:	nwhitehorn, julian

Modified:
  head/usr.sbin/bsdinstall/distextract/distextract.c
  head/usr.sbin/bsdinstall/distfetch/distfetch.c

Modified: head/usr.sbin/bsdinstall/distextract/distextract.c
==============================================================================
--- head/usr.sbin/bsdinstall/distextract/distextract.c	Wed Oct  1 18:07:34 2014	(r272378)
+++ head/usr.sbin/bsdinstall/distextract/distextract.c	Wed Oct  1 18:59:57 2014	(r272379)
@@ -40,45 +40,102 @@ __FBSDID("$FreeBSD$");
 #include <string.h>
 #include <unistd.h>
 
+/* Data to process */
+static char *distdir = NULL;
+struct file_node {
+	char	*path;
+	char	*name;
+	int	length;
+	struct file_node *next;
+};
+static struct file_node *dists = NULL;
+
+/* Function prototypes */
 static int	count_files(const char *file);
-static int	extract_files(int nfiles, const char **files);
+static int	extract_files(int nfiles, struct file_node *files);
+
+#if __FreeBSD_version <= 1000008 /* r232154: bump for libarchive update */
+#define archive_read_support_filter_all(x) \
+	archive_read_support_compression_all(x)
+#endif
+
+#define _errx(...) (end_dialog(), errx(__VA_ARGS__))
 
 int
 main(void)
 {
-	const char **dists;
-	char *diststring;
-	int i;
+	char *chrootdir;
+	char *distributions;
 	int ndists = 0;
 	int retval;
+	size_t file_node_size = sizeof(struct file_node);
+	size_t span;
+	struct file_node *dist = dists;
 	char error[PATH_MAX + 512];
 
-	if (getenv("DISTRIBUTIONS") == NULL)
+	if ((distributions = getenv("DISTRIBUTIONS")) == NULL)
 		errx(EXIT_FAILURE, "DISTRIBUTIONS variable is not set");
+	if ((distdir = getenv("BSDINSTALL_DISTDIR")) == NULL)
+		distdir = __DECONST(char *, "");
 
-	diststring = strdup(getenv("DISTRIBUTIONS"));
-	for (i = 0; diststring[i] != 0; i++)
-		if (isspace(diststring[i]) && !isspace(diststring[i+1]))
-			ndists++;
-	ndists++; /* Last one */
-
-	dists = calloc(ndists, sizeof(const char *));
-	if (dists == NULL) {
-		free(diststring);
-		errx(EXIT_FAILURE, "Out of memory!");
-	}
-
-	for (i = 0; i < ndists; i++)
-		dists[i] = strsep(&diststring, " \t");
-
+	/* Initialize dialog(3) */
 	init_dialog(stdin, stdout);
 	dialog_vars.backtitle = __DECONST(char *, "FreeBSD Installer");
 	dlg_put_backtitle();
 
-	if (chdir(getenv("BSDINSTALL_CHROOT")) != 0) {
+	dialog_msgbox("",
+	    "Checking distribution archives.\nPlease wait...", 4, 35, FALSE);
+
+	/*
+	 * Parse $DISTRIBUTIONS into linked-list
+	 */
+	while (*distributions != '\0') {
+		span = strcspn(distributions, "\t\n\v\f\r ");
+		if (span < 1) { /* currently on whitespace */
+			distributions++;
+			continue;
+		}
+		ndists++;
+
+		/* Allocate a new struct for the distribution */
+		if (dist == NULL) {
+			if ((dist = calloc(1, file_node_size)) == NULL)
+				_errx(EXIT_FAILURE, "Out of memory!");
+			dists = dist;
+		} else {
+			dist->next = calloc(1, file_node_size);
+			if (dist->next == NULL)
+				_errx(EXIT_FAILURE, "Out of memory!");
+			dist = dist->next;
+		}
+
+		/* Set path */
+		if ((dist->path = malloc(span + 1)) == NULL)
+			_errx(EXIT_FAILURE, "Out of memory!");
+		snprintf(dist->path, span + 1, "%s", distributions);
+		dist->path[span] = '\0';
+
+		/* Set display name */
+		dist->name = strrchr(dist->path, '/');
+		if (dist->name == NULL)
+			dist->name = dist->path;
+
+		/* Set initial length in files (-1 == error) */
+		dist->length = count_files(dist->path);
+		if (dist->length < 0) {
+			end_dialog();
+			return (EXIT_FAILURE);
+		}
+
+		distributions += span;
+	}
+
+	/* Optionally chdir(2) into $BSDINSTALL_CHROOT */
+	chrootdir = getenv("BSDINSTALL_CHROOT");
+	if (chrootdir != NULL && chdir(chrootdir) != 0) {
 		snprintf(error, sizeof(error),
-		    "Could could change to directory %s: %s\n",
-		    getenv("BSDINSTALL_DISTDIR"), strerror(errno));
+		    "Could not change to directory %s: %s\n",
+		    chrootdir, strerror(errno));
 		dialog_msgbox("Error", error, 0, 0, TRUE);
 		end_dialog();
 		return (EXIT_FAILURE);
@@ -88,20 +145,28 @@ main(void)
 
 	end_dialog();
 
-	free(diststring);
-	free(dists);
+	while ((dist = dists) != NULL) {
+		dists = dist->next;
+		if (dist->path != NULL)
+			free(dist->path);
+		free(dist);
+	}
 
 	return (retval);
 }
 
+/*
+ * Returns number of files in archive file. Parses $BSDINSTALL_DISTDIR/MANIFEST
+ * if it exists, otherwise uses archive(3) to read the archive file.
+ */
 static int
 count_files(const char *file)
 {
 	static FILE *manifest = NULL;
-	char *tok1;
-	char *tok2;
+	char *p;
 	int file_count;
 	int retval;
+	size_t span;
 	struct archive *archive;
 	struct archive_entry *entry;
 	char line[512];
@@ -109,36 +174,46 @@ count_files(const char *file)
 	char errormsg[PATH_MAX + 512];
 
 	if (manifest == NULL) {
-		snprintf(path, sizeof(path), "%s/MANIFEST",
-		    getenv("BSDINSTALL_DISTDIR"));
+		snprintf(path, sizeof(path), "%s/MANIFEST", distdir);
 		manifest = fopen(path, "r");
 	}
 
 	if (manifest != NULL) {
 		rewind(manifest);
 		while (fgets(line, sizeof(line), manifest) != NULL) {
-			tok2 = line;
-			tok1 = strsep(&tok2, "\t");
-			if (tok1 == NULL || strcmp(tok1, file) != 0)
+			p = &line[0];
+			span = strcspn(p, "\t") ;
+			if (span < 1 || strncmp(p, file, span) != 0)
 				continue;
 
 			/*
 			 * We're at the right manifest line. The file count is
 			 * in the third element
 			 */
-			tok1 = strsep(&tok2, "\t");
-			tok1 = strsep(&tok2, "\t");
-			if (tok1 != NULL)
-				return atoi(tok1);
+			span = strcspn(p += span + (*p != '\0' ? 1 : 0), "\t");
+			span = strcspn(p += span + (*p != '\0' ? 1 : 0), "\t");
+			if (span > 0) {
+				file_count = (int)strtol(p, (char **)NULL, 10);
+				if (file_count == 0 && errno == EINVAL)
+					continue;
+				return (file_count);
+			}
 		}
 	}
 
-	/* Either we didn't have a manifest, or this archive wasn't there */
-	archive = archive_read_new();
+	/*
+	 * Either no manifest, or manifest didn't mention this archive.
+	 * Use archive(3) to read the archive, counting files within.
+	 */
+	if ((archive = archive_read_new()) == NULL) {
+		snprintf(errormsg, sizeof(errormsg),
+		    "Error: %s\n", archive_error_string(NULL));
+		dialog_msgbox("Extract Error", errormsg, 0, 0, TRUE);
+		return (-1);
+	}
 	archive_read_support_format_all(archive);
 	archive_read_support_filter_all(archive);
-	snprintf(path, sizeof(path), "%s/%s", getenv("BSDINSTALL_DISTDIR"),
-	    file);
+	snprintf(path, sizeof(path), "%s/%s", distdir, file);
 	retval = archive_read_open_filename(archive, path, 4096);
 	if (retval != ARCHIVE_OK) {
 		snprintf(errormsg, sizeof(errormsg),
@@ -157,7 +232,7 @@ count_files(const char *file)
 }
 
 static int
-extract_files(int nfiles, const char **files)
+extract_files(int nfiles, struct file_node *files)
 {
 	int archive_file;
 	int archive_files[nfiles];
@@ -169,43 +244,51 @@ extract_files(int nfiles, const char **f
 	int total_files = 0;
 	struct archive *archive;
 	struct archive_entry *entry;
+	struct file_node *file;
 	char status[8];
+	static char title[] = "Archive Extraction";
+	static char pprompt[] = "Extracting distribution files...\n";
 	char path[PATH_MAX];
 	char errormsg[PATH_MAX + 512];
 	const char *items[nfiles*2];
 
 	/* Make the transfer list for dialog */
-	for (i = 0; i < nfiles; i++) {
-		items[i*2] = strrchr(files[i], '/');
-		if (items[i*2] != NULL)
-			items[i*2]++;
-		else
-			items[i*2] = files[i];
+	i = 0;
+	for (file = files; file != NULL; file = file->next) {
+		items[i*2] = file->name;
 		items[i*2 + 1] = "Pending";
-	}
-
-	dialog_msgbox("",
-	    "Checking distribution archives.\nPlease wait...", 0, 0, FALSE);
+		archive_files[i] = file->length;
 
-	/* Count all the files */
-	for (i = 0; i < nfiles; i++) {
-		archive_files[i] = count_files(files[i]);
-		if (archive_files[i] < 0)
-			return (-1);
-		total_files += archive_files[i];
+		total_files += file->length;
+		i++;
 	}
 
-	for (i = 0; i < nfiles; i++) {
-		archive = archive_read_new();
+	i = 0;
+	for (file = files; file != NULL; file = file->next) {
+		if ((archive = archive_read_new()) == NULL) {
+			snprintf(errormsg, sizeof(errormsg),
+			    "Error: %s\n", archive_error_string(NULL));
+			dialog_msgbox("Extract Error", errormsg, 0, 0, TRUE);
+			return (EXIT_FAILURE);
+		}
 		archive_read_support_format_all(archive);
 		archive_read_support_filter_all(archive);
-		snprintf(path, sizeof(path), "%s/%s",
-		    getenv("BSDINSTALL_DISTDIR"), files[i]);
+		snprintf(path, sizeof(path), "%s/%s", distdir, file->path);
 		retval = archive_read_open_filename(archive, path, 4096);
+		if (retval != 0) {
+			snprintf(errormsg, sizeof(errormsg),
+			    "Error opening %s: %s\n", file->name,
+			    archive_error_string(archive));
+			dialog_msgbox("Extract Error", errormsg, 0, 0, TRUE);
+			return (EXIT_FAILURE);
+		}
 
 		items[i*2 + 1] = "In Progress";
 		archive_file = 0;
 
+		dialog_mixedgauge(title, pprompt, 0, 0, progress, nfiles,
+		    __DECONST(char **, items));
+
 		while ((retval = archive_read_next_header(archive, &entry)) ==
 		    ARCHIVE_OK) {
 			last_progress = progress;
@@ -216,8 +299,7 @@ extract_files(int nfiles, const char **f
 			items[i*2 + 1] = status;
 
 			if (progress > last_progress)
-				dialog_mixedgauge("Archive Extraction",
-				    "Extracting distribution files...", 0, 0,
+				dialog_mixedgauge(title, pprompt, 0, 0,
 				    progress, nfiles,
 				    __DECONST(char **, items));
 
@@ -240,12 +322,16 @@ extract_files(int nfiles, const char **f
 			    "Error while extracting %s: %s\n", items[i*2],
 			    archive_error_string(archive));
 			items[i*2 + 1] = "Failed";
-			dialog_msgbox("Extract Error", errormsg, 0, 0,
-			    TRUE);
+			dialog_msgbox("Extract Error", errormsg, 0, 0, TRUE);
 			return (retval);
 		}
 
+		progress = (current_files*100)/total_files; 
+		dialog_mixedgauge(title, pprompt, 0, 0, progress, nfiles,
+		    __DECONST(char **, items));
+
 		archive_read_free(archive);
+		i++;
 	}
 
 	return (EXIT_SUCCESS);

Modified: head/usr.sbin/bsdinstall/distfetch/distfetch.c
==============================================================================
--- head/usr.sbin/bsdinstall/distfetch/distfetch.c	Wed Oct  1 18:07:34 2014	(r272378)
+++ head/usr.sbin/bsdinstall/distfetch/distfetch.c	Wed Oct  1 18:59:57 2014	(r272379)
@@ -82,7 +82,7 @@ main(void)
 		    getenv("BSDINSTALL_DISTDIR"), strerror(errno));
 		dialog_msgbox("Error", error, 0, 0, TRUE);
 		end_dialog();
-		return (1);
+		return (EXIT_FAILURE);
 	}
 
 	nfetched = fetch_files(ndists, urls);
@@ -94,7 +94,7 @@ main(void)
 		free(urls[i]);
 	free(urls);
 
-	return ((nfetched == ndists) ? 0 : 1);
+	return ((nfetched == ndists) ? EXIT_SUCCESS : EXIT_FAILURE);
 }
 
 static int



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201410011859.s91Ixvqa047151>