Date: Mon, 10 May 2010 04:01:06 GMT From: Garrett Cooper <gcooper@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 178022 for review Message-ID: <201005100401.o4A416bY015772@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@178022?ac=10 Change 178022 by gcooper@gcooper-bayonetta on 2010/05/10 04:00:45 1. Fix indentation for RESOLVE_PATH. 2. Tighten down -s use to reject relative paths from -p (that will only lead to wackiness and confusing use). 3. Fix BaseDir checks, and tack on `/' if needed. Affected files ... .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/main.c#8 edit .. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#8 edit Differences ... ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/main.c#8 (text+ko) ==== @@ -72,10 +72,12 @@ { NULL, 0, NULL, 0 }, }; -#define RESOLVE_PATH(v, path) \ - do { \ - if ((v = malloc(PATH_MAX)) == NULL || realpath(path, v) == NULL)\ - err(EXIT_FAILURE, "couldn't resolve path for %s", path);\ +#define RESOLVE_PATH(v, path) \ + do { \ + if ((v = malloc(PATH_MAX)) == NULL || \ + realpath(path, v) == NULL) \ + err(EXIT_FAILURE, "couldn't resolve path for %s",\ + path); \ } while (0) int @@ -83,6 +85,7 @@ { int ch; char **pkgs, **start, *tmp; + size_t basedir_len; pkg_wrap(PKG_INSTALL_VERSION, argv); @@ -124,17 +127,32 @@ break; case 's': - if (strlen(SrcDir) > PATH_MAX) - errx(EXIT_FAILURE, "-s argument invalid: %s", - strerror(ENAMETOOLONG)); - SrcDir = optarg; + RESOLVE_PATH(SrcDir, optarg); break; case 'S': - if (strlen(BaseDir) > PATH_MAX) + + /* + * Copy down BaseDir so we can realloc(3) the memory later if need + * be. + */ + if ((BaseDir = strdup(optarg)) == NULL) + err(EXIT_FAILURE, "couldn't copy -s argument"); + + basedir_len = strlen(BaseDir); + + if (basedir_len > PATH_MAX) errx(EXIT_FAILURE, "-s argument invalid: %s", strerror(ENAMETOOLONG)); - BaseDir = optarg; + /* No trailing '/' -- add one on. */ + else if (*(BaseDir+basedir_len-1) != '/') { + basedir_len++; + if ((BaseDir = realloc(BaseDir, basedir_len)) == NULL) + err(EXIT_FAILURE, "couldn't copy -s argument"); + if (strlcat(BaseDir, "/", PATH_MAX) > PATH_MAX) + errx(EXIT_FAILURE, "-s argument invalid: %s", + strerror(ENAMETOOLONG)); + } break; case 'f': ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#8 (text+ko) ==== @@ -24,6 +24,7 @@ #include <sys/types.h> #include <sys/mman.h> #include <sys/stat.h> +#include <assert.h> #include <err.h> #include <errno.h> #include <libgen.h> @@ -61,7 +62,10 @@ if (Verbose && !PlistOnly) printf("Creating package %s\n", pkg); - /* chop suffix off if already specified, remembering if we want to compress */ + /* + * chop suffix off if already specified, remembering if we want to + * compress. + */ len = strlen(pkg); if (len > 4) { if (!strcmp(&pkg[len - 4], ".tbz")) { @@ -153,10 +157,8 @@ if (ndeps != 0) { /* Create easy to use NULL-terminated list */ deps = alloca(sizeof(*deps) * ndeps + 1); - if (deps == NULL) { + if (deps == NULL) errx(2, "%s: alloca() failed", __func__); - /* Not reached */ - } for (i = 0; Pkgdeps;) { cp = strsep(&Pkgdeps, " \t\n"); if (*cp) { @@ -211,12 +213,9 @@ read_plist(&plist, pkg_in); /* Prefix should add an @cwd to the packing list */ - if (Prefix) { - char resolved_prefix[PATH_MAX]; - if (realpath(Prefix, resolved_prefix) == NULL) - err(EXIT_FAILURE, "couldn't resolve path for prefix: %s", Prefix); - add_plist_top(&plist, PLIST_CWD, resolved_prefix); - } + if (Prefix) + add_plist_top(&plist, PLIST_CWD, Prefix); + /* Add the origin if asked, at the top */ if (Origin) @@ -261,9 +260,9 @@ /* mark_plist(&plist); */ /* Now put the release specific items in */ - if (!Prefix) { + if (!Prefix) add_plist(&plist, PLIST_CWD, "."); - } + write_file(COMMENT_FNAME, Comment); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, COMMENT_FNAME); @@ -274,44 +273,37 @@ add_cksum(&plist, plist.tail, DESC_FNAME); if (Install) { - copy_file(home, Install, INSTALL_FNAME); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, INSTALL_FNAME); add_cksum(&plist, plist.tail, INSTALL_FNAME); } if (PostInstall) { - copy_file(home, PostInstall, POST_INSTALL_FNAME); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, POST_INSTALL_FNAME); add_cksum(&plist, plist.tail, POST_INSTALL_FNAME); } if (DeInstall) { - copy_file(home, DeInstall, DEINSTALL_FNAME); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, DEINSTALL_FNAME); add_cksum(&plist, plist.tail, DEINSTALL_FNAME); } if (PostDeInstall) { - copy_file(home, PostDeInstall, POST_DEINSTALL_FNAME); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, POST_DEINSTALL_FNAME); add_cksum(&plist, plist.tail, POST_DEINSTALL_FNAME); } if (Require) { - copy_file(home, Require, REQUIRE_FNAME); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, REQUIRE_FNAME); add_cksum(&plist, plist.tail, REQUIRE_FNAME); } if (Display) { - copy_file(home, Display, DISPLAY_FNAME); 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) { - copy_file(home, Mtree, MTREE_FNAME); add_plist(&plist, PLIST_IGNORE, NULL); add_plist(&plist, PLIST_FILE, MTREE_FNAME); add_cksum(&plist, plist.tail, MTREE_FNAME); @@ -322,14 +314,13 @@ fp = fopen(CONTENTS_FNAME, "w"); if (!fp) { cleanup(0); - errx(2, "%s: can't open file %s for writing", - __func__, CONTENTS_FNAME); + errx(2, "%s: can't open file %s for writing", __func__, + CONTENTS_FNAME); } write_plist(&plist, fp); if (fclose(fp)) { cleanup(0); - errx(2, "%s: error while closing %s", - __func__, CONTENTS_FNAME); + errx(2, "%s: error while closing %s", __func__, CONTENTS_FNAME); } /* And stick it into a tar ball */ @@ -348,10 +339,10 @@ { /* XXX (gcooper): add acl and xattr support? */ -#define ADD_FILE(src_file, dest_file, archive_entry_open_flags) \ +#define ADD_FILE(srcfile, destfile, archive_entry_open_flags) \ if (error == NULL) { \ - if ((archive_entry_fd = open(src_file, \ - archive_entry_fd)) == -1 || \ + 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, \ @@ -363,6 +354,7 @@ 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);\ @@ -379,17 +371,16 @@ close(archive_entry_fd); \ } -#if NOTYET PackingList p; -#endif struct stat *sb; struct archive *archive = NULL; struct archive_entry *entry = NULL; + char *destbase = NULL; + char *destfile = NULL; + char *prefix = NULL; + char *srcbase = NULL; + char *srcfile = NULL; char tball[PATH_MAX]; -#if NOTYET - char *prefix = NULL; -#endif - char *starting_point = NULL; const char *cname = NULL; const char *error = NULL; @@ -397,16 +388,16 @@ 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; - Boolean passed = FALSE; - if (*pkg == '/') snprintf(tball, sizeof(tball), "%s.%s", pkg, suff); else snprintf(tball, sizeof(tball), "%s/%s.%s", homedir, pkg, suff); - archive_entry_open_flags = O_RDONLY; + archive_entry_open_flags = archive_metadata_open_flags = O_RDONLY; if (Dereference == FALSE) archive_entry_open_flags |= O_NOFOLLOW; @@ -416,8 +407,12 @@ if (Regenerate == FALSE) archive_open_flags |= O_CREAT; - if ((starting_point = getwd(NULL)) == NULL) - warn("%s: failed to determine current directory", __func__); + if ((destfile = calloc(PATH_MAX, sizeof(char))) == NULL || + (destbase = calloc(PATH_MAX, sizeof(char))) == NULL || + (prefix = calloc(PATH_MAX, sizeof(char))) == NULL || + (srcbase = calloc(PATH_MAX, sizeof(char))) == NULL || + (srcfile = calloc(PATH_MAX, sizeof(char))) == NULL) + error = strerror(errno); /* * If the package tarball exists already, and we are running in * `no clobber' mode, skip this package. @@ -460,9 +455,9 @@ } - if (error != NULL) - if (archive_write_open_fd(archive, archive_fd) != ARCHIVE_OK) - error = archive_error_string(archive); + if (error != NULL && + archive_write_open_fd(archive, archive_fd) != ARCHIVE_OK) + error = archive_error_string(archive); if (error != NULL) { #ifdef NOTYET @@ -478,91 +473,194 @@ cname, tball); ADD_FILE(CONTENTS_FNAME, CONTENTS_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + archive_metadata_open_flags); ADD_FILE(COMMENT_FNAME, COMMENT_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + archive_metadata_open_flags); ADD_FILE(DESC_FNAME, DESC_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + archive_metadata_open_flags); if (Install) - ADD_FILE(INSTALL_FNAME, INSTALL_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(Install, INSTALL_FNAME, + archive_metadata_open_flags); if (PostInstall) - ADD_FILE(POST_INSTALL_FNAME, POST_INSTALL_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(PostInstall, POST_INSTALL_FNAME, + archive_metadata_open_flags); if (DeInstall) - ADD_FILE(DEINSTALL_FNAME, DEINSTALL_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(DeInstall, DEINSTALL_FNAME, + archive_metadata_open_flags); if (PostDeInstall) - ADD_FILE(POST_DEINSTALL_FNAME, POST_DEINSTALL_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(PostDeInstall, POST_DEINSTALL_FNAME, + archive_metadata_open_flags); if (Require) - ADD_FILE(REQUIRE_FNAME, REQUIRE_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(Require, REQUIRE_FNAME, + archive_metadata_open_flags); if (Display) - ADD_FILE(DISPLAY_FNAME, DISPLAY_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(Display, DISPLAY_FNAME, + archive_metadata_open_flags); if (Mtree) - ADD_FILE(MTREE_FNAME, MTREE_FNAME, - archive_entry_open_flags ^ O_NOFOLLOW); + ADD_FILE(Mtree, MTREE_FNAME, + archive_metadata_open_flags); + + } + + for (p = plist->head; error == NULL && p != NULL; p = p->next) + switch(p->type) { + case PLIST_FILE: + + /* Add a file to the archive. */ + + /* XXX: plist bug -- empty line */ + assert (p->name != NULL); + + destfile[0] = srcfile[0] = '\0'; + + if (destbase != NULL) + if (strlcat(destfile, destbase, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + if (error != NULL) + if (strlcat(destfile, p->name, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + if (srcbase != NULL) + if (strlcat(srcfile, srcbase, + PATH_MAX) > PATH_MAX) + error = strerror(ENAMETOOLONG); + + if (error != NULL) + if (strlcat(srcfile, p->name, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); - passed = TRUE; + ADD_FILE(srcfile, destfile, archive_entry_open_flags); -#if NOTYET - /* - * XXX (gcooper): Fix style(9) for for-loop after changes have - * stabilized. - */ - for (p = plist->head; p != NULL; p = p->next) { - switch(p->type) { - case PLIST_FILE: - /* Add p->name to archive. */ - break; - case PLIST_CWD: - if (p->name != NULL) { - /* - * Add <base>/<@cwd dir> - * to archive. - */ - if (BaseDir != NULL && - p->name[0] == '/') ; - /* else, chdir(<prefix>) . */ - if (prefix == NULL) - prefix = p->name; + break; - } + case PLIST_CWD: - /* FALLTHROUGH */ - case PLIST_SRC: + if (p->name == NULL) { /* - * 1. chdir(<base>). - * 2. Add the <base>/<@cwd-dir>. + * Broken plist; prefix must be defined before + * calling `@cwd' with variable following + * whitespace. */ - break; - default: - /* Catch-all for the rest of the cases. */ - break; + assert(prefix != NULL); + + /* Reset destbase */ + if (strlcpy(destbase, prefix, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + /* Reset srcbase */ + if (strlcpy(srcbase, BaseDir, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + else if (strlcpy(srcbase, prefix, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + } else { + + /* Tack BaseDir on the front if defined. */ + if (BaseDir != NULL) { + if (strlcpy(srcbase, BaseDir, + PATH_MAX) > PATH_MAX) + error = strerror(ENAMETOOLONG); + } else + srcbase[0] = '\0'; + + if (strlcat(destbase, p->name, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + else if (strlcat(srcbase, p->name, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + /* First @cwd statement. */ + if (prefix == NULL) + prefix = p->name; + + } + + /* + * Tack on '/' if necessary. It's easier to do it + * once here instead of n times above when adding the + * data to file prefixes. + */ + if (error == NULL) { + +#define ADD_TRAILING_SLASH(v, v_len) \ + do { \ + v_len = strlen(v); \ + if (v[v_len-1] != '/') { \ + if (v_len >= PATH_MAX) \ + error = strerror(ENAMETOOLONG); \ + else { \ + v[v_len] = '/'; \ + v[v_len+1] = '\0'; \ + } \ + } \ + } while (0) + + ADD_TRAILING_SLASH(destbase, destbase_len); + ADD_TRAILING_SLASH(srcbase, srcbase_len); + } + + break; + + case PLIST_SRC: + + /* + * plist bug, i.e. `@srcdir' (or any trailing + * whitespace variants of that. + * + * XXX (gcooper): the check should be moved to + * libpkg/plist.c . + */ + assert(p->name != NULL); + + /* + * Set the appropriate prefix for the next file to + * install (only for @srcdir). + * + * Reset to '' + */ + if (strlcpy(srcbase, p->name, PATH_MAX) > + PATH_MAX) + error = strerror(ENAMETOOLONG); + + break; + + default: + /* Catch-all for the rest of the cases. */ + break; } -#endif - - } if (archive != NULL) archive_write_finish(archive); - if (error != NULL) { - passed = FALSE; + if (error != NULL) warnx("%s: unable to create the package '%s': %s", __func__, tball, error); - } if (0 <= archive_fd) { close(archive_fd); - if (passed == FALSE && unlink(tball) == -1) + if (error != NULL && unlink(tball) == -1) warn("%s: failed to remove incomplete package - '%s'", __func__, tball); } - if (starting_point != NULL) - free(starting_point); + if (destbase != NULL) + free(destbase); + if (destfile != NULL) + free(destfile); + if (srcbase != NULL) + free(srcbase); + if (srcfile != NULL) + free(srcfile); + if (destfile != NULL) + free(destfile); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201005100401.o4A416bY015772>