From owner-p4-projects@FreeBSD.ORG Sat May 15 08:13:19 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 4C6BE1065670; Sat, 15 May 2010 08:13:19 +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 ECB1A106566B for ; Sat, 15 May 2010 08:13:18 +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 D8CDE8FC17 for ; Sat, 15 May 2010 08:13:18 +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 o4F8DIx5053366 for ; Sat, 15 May 2010 08:13:18 GMT (envelope-from gcooper@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o4F8DIpY053364 for perforce@freebsd.org; Sat, 15 May 2010 08:13:18 GMT (envelope-from gcooper@FreeBSD.org) Date: Sat, 15 May 2010 08:13:18 GMT Message-Id: <201005150813.o4F8DIpY053364@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 178293 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 08:13:19 -0000 http://p4web.freebsd.org/@@178293?ac=10 Change 178293 by gcooper@starr-bastion on 2010/05/15 08:13:07 GOOD BYE AND GOOD RIDDANCE BUFFERED read_plist! This is one big(ish) step towards a sane solution. Affected files ... .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/file.c#13 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/pkg.h#7 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/plist.c#2 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/add/perform.c#10 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#26 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/delete/perform.c#4 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/info/perform.c#5 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/version/perform.c#9 edit Differences ... ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/file.c#13 (text+ko) ==== @@ -356,13 +356,13 @@ unpack_to_buffer(const char *pkg, const char *file) { - FILE *fd = NULL; + struct stat sb; char *buf = NULL; - struct stat sb; + int fd; - if ((fd = unpack_to_fd(pkg, file)) != NULL) { + if ((fd = unpack_to_fd(pkg, file)) != -1) { - if (fstat(fileno(fd), &sb) == 0) { + if (fstat(fd, &sb) == 0) { /* * User either passed in a non-NULL value or we need @@ -371,15 +371,16 @@ */ buf = malloc(sb.st_size); if (buf != NULL) { - if (fread(buf, sb.st_size, 1, fd) != - sb.st_size) { + if (read(fd, buf, sb.st_size) != sb.st_size) free(buf); - } } } } + if (fd != -1) + close(fd); + return buf; } @@ -504,21 +505,9 @@ * 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. - * - * [The return code info will eventually be...] - * * Return -1 on failure, a value greater than 0 on success. */ -FILE* +int unpack_to_fd(const char *pkg, const char *file) { struct archive *archive; @@ -529,7 +518,7 @@ const char *error = NULL; const char *pkg_name_humanized; - FILE *fd = NULL; + int fd = -1; /* int fd = -1; */ int archive_fd = -1, r; @@ -592,7 +581,7 @@ if (Verbose) printf("X - %s\n", entry_pathname); - fd = fopen(entry_pathname, "r"); + fd = open(entry_pathname, O_RDONLY); } else { error = archive_error_string(archive); warnx("%s: extraction for %s failed: " ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/pkg.h#7 (text+ko) ==== @@ -190,7 +190,7 @@ int delete_hierarchy(const char *, Boolean, Boolean); char* unpack_to_buffer(const char *, const char *); int unpack_to_disk(const char *, const char *); -FILE* unpack_to_fd(const char *, const char *); +int unpack_to_fd(const char *, const char *); void format_cmd(char *, int, const char *, const char *, const char *); /* Msg */ @@ -212,7 +212,7 @@ void add_plist_top(Package *, plist_t, const char *); void delete_plist(Package *pkg, Boolean all, plist_t type, const char *name); void write_plist(Package *, FILE *); -void read_plist(Package *, FILE *); +void read_plist(Package *, int); int plist_cmd(const char *, char **); int delete_package(Boolean, Boolean, Package *); Boolean make_preserve_name(char *, int, const char *, const char *); ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/lib/libpkg/plist.c#2 (text+ko) ==== @@ -21,10 +21,14 @@ #include __FBSDID("$FreeBSD: src/lib/libpkg/plist.c,v 1.1 2010/04/23 11:07:43 flz Exp $"); -#include "pkg.h" +#include +#include +#include #include #include +#include "pkg.h" + /* Add an item to a packing list */ void add_plist(Package *p, plist_t type, const char *arg) @@ -257,7 +261,7 @@ /* Read a packing list from a file */ void -read_plist(Package *pkg, FILE *fp) +read_plist(Package *pkg, int fd) { char *cp, pline[FILENAME_MAX]; int cmd, major, minor; @@ -265,7 +269,9 @@ pkg->fmtver_maj = 1; pkg->fmtver_mnr = 0; pkg->origin = NULL; - while (fgets(pline, FILENAME_MAX, fp)) { + + /* XXX (gcooper): BAD BAD BAD -- this can be longer than FILENAME_MAX */ + while (0 < read(fd, pline, FILENAME_MAX)) { int len = strlen(pline); while (len && isspace(pline[len - 1])) ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/add/perform.c#10 (text+ko) ==== @@ -71,7 +71,7 @@ char **matched; char *extract; const char *where_to; - FILE *cfile; + int cfile; int code; int inPlace, conflictsfound, errcode; /* support for separate pre/post install scripts */ @@ -95,7 +95,7 @@ warnx("pkg_add in SLAVE mode can't chdir to %s", playpen); return 1; } - read_plist(&Plist, stdin); + read_plist(&Plist, fileno(stdin)); where_to = playpen; } /* Nope - do it now */ @@ -106,15 +106,15 @@ warnx("unable to fetch '%s' by URL", pkg); return 1; } - cfile = fopen(CONTENTS_FNAME, "r"); - if (!cfile) { + cfile = open(CONTENTS_FNAME, O_RDONLY); + if (cfile == -1) { warnx( "unable to open table of contents file '%s' - not a package?", CONTENTS_FNAME); goto bomb; } read_plist(&Plist, cfile); - fclose(cfile); + close(cfile); } else { @@ -140,7 +140,7 @@ setenv("_TOP", where_to, 1); if (extract_whole_archive_from_stdin == TRUE) { if (unpack_to_disk(NULL, NULL) == 0) - cfile = fopen(CONTENTS_FNAME, "r"); + cfile = open(CONTENTS_FNAME, O_RDONLY); else { warnx("unable to extract table of contents file from '%s' " "- not a package?", pkg); @@ -149,14 +149,14 @@ } else cfile = unpack_to_fd(pkg, CONTENTS_FNAME); - if (!cfile) { + if (cfile == -1) { warnx( "unable to open table of contents file '%s' - not a package?", CONTENTS_FNAME); goto bomb; } read_plist(&Plist, cfile); - fclose(cfile); + close(cfile); /* Extract directly rather than moving? Oh goodie! */ if (find_plist_option(&Plist, "extract-in-place")) { ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#26 (text+ko) ==== @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -55,12 +56,13 @@ int pkg_perform(char **pkgs) { + Package plist; + FILE *fp; static const char *home; char *pkg = *pkgs; /* Only one arg to create */ char *cp; - FILE *pkg_in, *fp; - Package plist; int len; + int pkg_in; const char *suf; /* Preliminary setup */ @@ -135,10 +137,10 @@ get_dash_string(&Comment); get_dash_string(&Desc); if (!strcmp(Contents, "-")) - pkg_in = stdin; + pkg_in = fileno(stdin); else { - pkg_in = fopen(Contents, "r"); - if (!pkg_in) { + pkg_in = open(Contents, O_RDONLY); + if (pkg_in == -1) { cleanup(0); errx(2, "%s: unable to open contents file '%s' for input", __func__, Contents); @@ -770,10 +772,10 @@ static int create_from_installed_recursive(const char *pkg, const char *suf) { - FILE *fp; Package plist; PackingList p; char tmp[PATH_MAX]; + int fd; int rval; if (!create_from_installed(InstalledPkg, pkg, suf)) @@ -786,13 +788,13 @@ } /* Suck in the contents list */ plist.head = plist.tail = NULL; - fp = fopen(tmp, "r"); - if (fp == NULL) { - warnx("unable to open %s file", tmp); + fd = open(tmp, O_RDONLY); + if (fd == -1) { + warn("unable to open %s file", tmp); return FALSE; } - read_plist(&plist, fp); - fclose(fp); + read_plist(&plist, fd); + close(fd); rval = TRUE; for (p = plist.head; p != NULL && rval == TRUE; p = p->next) { if (p->type == PLIST_PKGDEP) { @@ -809,7 +811,7 @@ static int create_from_installed(const char *ipkg, const char *pkg, const char *suf) { - FILE *fp; + int fd; Package plist; char homedir[MAXPATHLEN], log_dir[FILENAME_MAX]; @@ -825,13 +827,13 @@ } /* Suck in the contents list */ plist.head = plist.tail = NULL; - fp = fopen(CONTENTS_FNAME, "r"); - if (!fp) { + fd = open(CONTENTS_FNAME, O_RDONLY); + if (fd == -1) { warnx("unable to open %s file", CONTENTS_FNAME); return FALSE; } - read_plist(&plist, fp); - fclose(fp); + read_plist(&plist, fd); + (void) close(fd); Install = isfile(INSTALL_FNAME) ? (char *)INSTALL_FNAME : NULL; PostInstall = isfile(POST_INSTALL_FNAME) ? ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/delete/perform.c#4 (text+ko) ==== @@ -121,9 +121,9 @@ static int pkg_do(char *pkg) { - FILE *cfile; char *deporigin, **deporigins = NULL, **depnames = NULL, ***depmatches, home[FILENAME_MAX]; PackingList p; + int cfile; int i, len; int isinstalled; /* support for separate pre/post install scripts */ @@ -202,9 +202,9 @@ } sanity_check(LogDir); - cfile = fopen(CONTENTS_FNAME, "r"); + cfile = open(CONTENTS_FNAME, O_RDONLY); - if (!cfile) { + if (cfile == -1) { warnx("unable to open '%s' file", CONTENTS_FNAME); return 1; } @@ -213,7 +213,7 @@ if (Prefix) add_plist(&Plist, PLIST_CWD, Prefix); read_plist(&Plist, cfile); - fclose(cfile); + (void) close(cfile); p = find_plist(&Plist, PLIST_CWD); if (!p) { ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/info/perform.c#5 (text+ko) ==== @@ -21,10 +21,13 @@ #include __FBSDID("$FreeBSD: src/usr.sbin/pkg_install/info/perform.c,v 1.57 2010/04/23 11:07:43 flz Exp $"); +#include +#include +#include + #include + #include "info.h" -#include -#include static int pkg_do(char *); static int find_pkg(struct which_head *); @@ -92,10 +95,10 @@ char log_dir[FILENAME_MAX]; char fname[FILENAME_MAX]; Package plist; - FILE *fp; struct stat sb; const char *cp = NULL; int code = 0; + int fd = -1; if (isURL(pkg)) { if ((cp = fileGetURL(NULL, pkg, KeepPackage)) != NULL) { @@ -164,15 +167,15 @@ /* Suck in the contents list */ plist.head = plist.tail = NULL; - fp = fopen(CONTENTS_FNAME, "r"); - if (!fp) { + fd = open(CONTENTS_FNAME, O_RDONLY); + if (fd == -1) { warnx("unable to open %s file", CONTENTS_FNAME); code = 1; goto bail; } /* If we have a prefix, add it now */ - read_plist(&plist, fp); - fclose(fp); + read_plist(&plist, fd); + close(fd); /* * Index is special info type that has to override all others to make @@ -368,7 +371,7 @@ return errcode; for (i = 0; installed[i] != NULL; i++) { - FILE *fp; + int fd; Package pkg; PackingList itr; char *cwd = NULL; @@ -376,15 +379,15 @@ snprintf(tmp, PATH_MAX, "%s/%s/%s", LOG_DIR, installed[i], CONTENTS_FNAME); - fp = fopen(tmp, "r"); - if (fp == NULL) { + fd = open(tmp, O_RDONLY); + if (fd == -1) { warn("%s", tmp); return 1; } pkg.head = pkg.tail = NULL; - read_plist(&pkg, fp); - fclose(fp); + read_plist(&pkg, fd); + close(fd); for (itr = pkg.head; itr != pkg.tail; itr = itr->next) { if (itr->type == PLIST_CWD) { cwd = itr->name; ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/version/perform.c#9 (text+ko) ==== @@ -145,23 +145,23 @@ static int pkg_do(char *pkg) { - char *ch, tmp[PATH_MAX], tmp2[PATH_MAX], *latest = NULL; Package plist; struct index_entry *ie; - FILE *fp; + char *ch, tmp[PATH_MAX], tmp2[PATH_MAX], *latest = NULL; + int fd; size_t len; /* Suck in the contents list. */ plist.head = plist.tail = NULL; plist.name = plist.origin = NULL; snprintf(tmp, PATH_MAX, "%s/%s/%s", LOG_DIR, pkg, CONTENTS_FNAME); - fp = fopen(tmp, "r"); - if (!fp) { + fd = open(tmp, O_RDONLY); + if (fd == -1) { warnx("the package info for package '%s' is corrupt", pkg); return 1; } - read_plist(&plist, fp); - fclose(fp); + read_plist(&plist, fd); + close(fd); if (plist.name == NULL) { warnx("%s does not appear to be a valid package!", pkg); return 1;