From owner-p4-projects@FreeBSD.ORG Thu May 13 03:38:23 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 046DB1065675; Thu, 13 May 2010 03:38:23 +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 AF020106566C for ; Thu, 13 May 2010 03:38:22 +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 9AC038FC22 for ; Thu, 13 May 2010 03:38:22 +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 o4D3cMp1026839 for ; Thu, 13 May 2010 03:38:22 GMT (envelope-from gcooper@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o4D3cMms026837 for perforce@freebsd.org; Thu, 13 May 2010 03:38:22 GMT (envelope-from gcooper@FreeBSD.org) Date: Thu, 13 May 2010 03:38:22 GMT Message-Id: <201005130338.o4D3cMms026837@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 178188 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: Thu, 13 May 2010 03:38:23 -0000 http://p4web.freebsd.org/@@178188?ac=10 Change 178188 by gcooper@starr-bastion on 2010/05/13 03:38:14 - Take the useful bits out of the ADD_FILE macro and move it into a real function called add_file. - Clean up create_from_installed_recursive so that it's style-compliant and the flow is easier to follow. - Fix some delightful bugs with wild pointers because it's been sufficiently long since I've used *stat and I made the time old mistake of not allocating sb beforehand, etc, etc. Thanks to the folks who replied on hackers@ for noting my stupid mistake. - Need to determine what to do about adding +CONTENTS, etc - Disable mmap(2) for now because it doesn't work; need to determine why not. Need to determine what to do about adding +CONTENTS, etc in make_dist since it's added twice today if Prefix is NULL and we're bootstrapping a package which isn't installed on the disk :(. The metadata files should be at the top of the archive because it would speed up pkg_info, and regular metadata reads when reading from a package archive opposed to reading data directly off the disk in $PKG_PREFIX. Affected files ... .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#20 edit Differences ... ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#20 (text+ko) ==== @@ -43,6 +43,8 @@ #include "create.h" static void sanity_check(void); +static const char* add_file(struct archive *, const char *, const char *, + const int); static void make_dist(const char *, const char *, const char *, Package *); static int create_from_installed_recursive(const char *, const char *); static int create_from_installed(const char *, const char *, const char *); @@ -262,7 +264,7 @@ /* mark_plist(&plist); */ /* Now put the release specific items in */ - if (!Prefix) + if (Prefix == NULL) add_plist(&plist, PLIST_CWD, "."); write_file(COMMENT_FNAME, Comment); @@ -274,38 +276,38 @@ add_plist(&plist, PLIST_FILE, DESC_FNAME); add_cksum(&plist, plist.tail, DESC_FNAME); - if (Install) { + if (Install != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, INSTALL_FNAME); add_cksum(&plist, plist.tail, INSTALL_FNAME); } - if (PostInstall) { + if (PostInstall != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, POST_INSTALL_FNAME); add_cksum(&plist, plist.tail, POST_INSTALL_FNAME); } - if (DeInstall) { + if (DeInstall != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, DEINSTALL_FNAME); add_cksum(&plist, plist.tail, DEINSTALL_FNAME); } - if (PostDeInstall) { + if (PostDeInstall != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, POST_DEINSTALL_FNAME); add_cksum(&plist, plist.tail, POST_DEINSTALL_FNAME); } - if (Require) { + if (Require != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, REQUIRE_FNAME); add_cksum(&plist, plist.tail, REQUIRE_FNAME); } - if (Display) { + if (Display != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, DISPLAY_FNAME); add_cksum(&plist, plist.tail, DISPLAY_FNAME); add_plist(&plist, PLIST_DISPLAY, DISPLAY_FNAME); } - if (Mtree) { + if (Mtree != NULL) { add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, MTREE_FNAME); add_cksum(&plist, plist.tail, MTREE_FNAME); @@ -324,7 +326,7 @@ err(2, "%s: error occurred when closing %s", __func__, CONTENTS_FNAME); } - /* And stick it into a tar ball */ + /* And stick it into a tarball */ make_dist(home, pkg, suf, &plist); /* Cleanup */ @@ -335,47 +337,110 @@ return TRUE; /* Success */ } +static const char* +add_file(struct archive *archive, const char *srcfile, const char *destfile, + const int archive_entry_open_flags) +{ + +/* + * XXX (gcooper): + * The mmap code below doesn't function with archive(3) today; need to + * determine why as no diags are fed back as to why it doesn't work, because + * it would probably be a performance boost to map the pages for the backing + * files into memory and just do one read as opposed to many small reads. + * + * #define BROKEN_MMAP + */ + + struct archive_entry *entry = NULL; + struct stat sb; + const char *error = NULL; + int archive_entry_fd; +#ifdef BROKEN_MMAP + void *archive_entry_map_addr = NULL; +#else + char archive_entry_buf[BUFSIZ]; + size_t len; +#endif + + if (Verbose) + printf("C - %s\n", srcfile); + + if ((archive_entry_fd = open(srcfile, archive_entry_open_flags)) == -1) + error = strerror(errno); + else if (fstat(archive_entry_fd, &sb) == -1) + error = strerror(errno); +#ifdef BROKEN_MMAP + else if ((archive_entry_map_addr = mmap(NULL, sb.st_size, PROT_READ, + MAP_SHARED, archive_entry_fd, 0)) == NULL) + error = strerror(errno); +#endif + else { + + if ((entry = archive_entry_new()) == NULL) + error = archive_error_string(archive); + else { + + archive_entry_copy_stat(entry, &sb); + archive_entry_copy_pathname(entry, destfile); + if (archive_write_header(archive, entry) != ARCHIVE_OK) + error = archive_error_string(archive); +#ifdef BROKEN_MMAP + /* + * XXX (gcooper): fails to write data here with mmap(2) + * enabled. + */ + else if (archive_write_data(archive, + archive_entry_map_addr, sb.st_size) != ARCHIVE_OK) + error = archive_error_string(archive); + (void) munmap(archive_entry_map_addr, sb.st_size); +#else + else if (error != NULL) { + + do { + + len = read(archive_entry_fd, + archive_entry_buf, + sizeof(archive_entry_buf)); + + if (archive_write_data(archive, + archive_entry_buf, + sizeof(archive_entry_buf)) != + ARCHIVE_OK) + error = archive_error_string(archive); + + } while (error == NULL && len > 0); + + } + +#endif + archive_entry_free(entry); + + } + + } + + if (0 <= archive_entry_fd) + close(archive_entry_fd); + + return error; + +} + static void make_dist(const char *homedir, const char *pkg, const char *suff, Package *plist) { - /* XXX (gcooper): add acl and xattr support? */ -#define ADD_FILE(srcfile, destfile, archive_entry_open_flags) \ - if (error == NULL) { \ - if ((archive_entry_fd = open(srcfile, \ - archive_entry_open_flags)) == -1 || \ - fstat(archive_entry_fd, sb) == -1) { \ - error = strerror(errno); \ - } else if ((archive_entry_map_addr = mmap(NULL, \ - PROT_READ, sb->st_size, MAP_SHARED, \ - archive_entry_fd, 0)) == NULL) { \ - error = strerror(errno); \ - } else { \ - if ((entry = archive_entry_new()) == NULL) \ - error = archive_error_string(archive); \ - else { \ - archive_entry_copy_stat(entry, sb); \ - archive_entry_copy_pathname(entry, destfile); \ - if (archive_write_header(archive, \ - entry) != ARCHIVE_OK) \ - error = archive_error_string(archive);\ - else if (archive_write_data(archive, \ - archive_entry_map_addr, \ - sb->st_size) != ARCHIVE_OK) \ - error = archive_error_string(archive);\ - (void) munmap(archive_entry_map_addr, \ - sb->st_size); \ - archive_entry_free(entry); \ - } \ - } \ - if (0 <= archive_entry_fd) \ - close(archive_entry_fd); \ - } +#define ADD_FILE(SRCFILE, DESTFILE, OPEN_FLAGS) \ + do { \ + if (error == NULL) { \ + error = add_file(archive, SRCFILE, DESTFILE, \ + OPEN_FLAGS); \ + } \ + } while (0) PackingList p; - struct stat *sb; struct archive *archive = NULL; - struct archive_entry *entry = NULL; struct lafe_matching *match_patterns = NULL; char *destbase = NULL; char *destfile = NULL; @@ -388,11 +453,9 @@ const char *error = NULL; int archive_fd = -1; int archive_open_flags; - int archive_entry_fd = -1; int archive_entry_open_flags; int archive_metadata_open_flags; int destbase_len, srcbase_len; - void *archive_entry_map_addr; if (*pkg == '/') snprintf(tball, sizeof(tball), "%s.%s", pkg, suff); @@ -740,41 +803,40 @@ static int create_from_installed_recursive(const char *pkg, const char *suf) { - FILE *fp; - Package plist; - PackingList p; - char tmp[PATH_MAX]; - int rval; + FILE *fp; + Package plist; + PackingList p; + char tmp[PATH_MAX]; + int rval; - if (!create_from_installed(InstalledPkg, pkg, suf)) - return FALSE; - snprintf(tmp, sizeof(tmp), "%s/%s/%s", LOG_DIR, InstalledPkg, CONTENTS_FNAME); - if (!fexists(tmp)) { - warnx("can't find package '%s' installed!", InstalledPkg); - return FALSE; - } - /* Suck in the contents list */ - plist.head = plist.tail = NULL; - fp = fopen(tmp, "r"); - if (!fp) { - warnx("unable to open %s file", tmp); - return FALSE; - } - read_plist(&plist, fp); - fclose(fp); - rval = TRUE; - for (p = plist.head; p ; p = p->next) { - if (p->type != PLIST_PKGDEP) - continue; - if (Verbose) - printf("Creating package %s\n", p->name); - if (!create_from_installed(p->name, p->name, suf)) { - rval = FALSE; - break; + if (!create_from_installed(InstalledPkg, pkg, suf)) + return FALSE; + snprintf(tmp, sizeof(tmp), "%s/%s/%s", + LOG_DIR, InstalledPkg, CONTENTS_FNAME); + if (!fexists(tmp)) { + warnx("can't find package '%s' installed!", InstalledPkg); + return FALSE; + } + /* Suck in the contents list */ + plist.head = plist.tail = NULL; + fp = fopen(tmp, "r"); + if (fp == NULL) { + warnx("unable to open %s file", tmp); + return FALSE; + } + read_plist(&plist, fp); + fclose(fp); + rval = TRUE; + for (p = plist.head; p != NULL && rval == TRUE; p = p->next) { + if (p->type == PLIST_PKGDEP) { + if (Verbose) + printf("Creating package %s\n", p->name); + if (!create_from_installed(p->name, p->name, suf)) + rval = FALSE; + } } - } - free_plist(&plist); - return rval; + free_plist(&plist); + return rval; } static int