From b1da57a21d9f5aa8e6e10c809e10aa0060f3fad5 Mon Sep 17 00:00:00 2001 From: dteske Date: Wed, 1 Oct 2014 18:59:57 +0000 Subject: [PATCH] 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 --- usr.sbin/bsdinstall/distextract/distextract.c | 218 ++++++++++++------ usr.sbin/bsdinstall/distfetch/distfetch.c | 4 +- 2 files changed, 154 insertions(+), 68 deletions(-) diff --git a/usr.sbin/bsdinstall/distextract/distextract.c b/usr.sbin/bsdinstall/distextract/distextract.c index 8e02d79cef97..54e0171cfe90 100644 --- a/usr.sbin/bsdinstall/distextract/distextract.c +++ b/usr.sbin/bsdinstall/distextract/distextract.c @@ -40,45 +40,102 @@ __FBSDID("$FreeBSD$"); #include #include +/* 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 **files) 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"; + archive_files[i] = file->length; + + total_files += file->length; + i++; } - dialog_msgbox("", - "Checking distribution archives.\nPlease wait...", 0, 0, FALSE); - - /* 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]; - } - - 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 **files) 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 **files) "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); diff --git a/usr.sbin/bsdinstall/distfetch/distfetch.c b/usr.sbin/bsdinstall/distfetch/distfetch.c index 69ff1d087dd5..4e870c8bbdbf 100644 --- a/usr.sbin/bsdinstall/distfetch/distfetch.c +++ b/usr.sbin/bsdinstall/distfetch/distfetch.c @@ -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