PERFORCE change 179079 for review
Garrett Cooper
gcooper at FreeBSD.org
Wed Jun 2 09:52:33 UTC 2010
http://p4web.freebsd.org/@@179079?ac=10
Change 179079 by gcooper at gcooper-bayonetta on 2010/06/02 09:51:53
Improve the overall flow of pkg_create as follows:
1. Only check for whether or not prefix is NULL once.
2. Add the prefix @cwd once in the application's lifetime instead of in
two separate instances.
3. Don't add the metadata files to the plist. Why? Because a) they
don't end up in any of the plists (installed from ports at least),
and b) it causes functional issues and introduces unneeded
complexity with the overall function of the package because one
needs to keep track of what files are the metadata files, where they
need to go (it's fluid because of the fact that one can specify
$PKG_DBDIR).
4. Consolidate all of the make_dist macros at the top of the function.
5. Fix some style in perform.c's function headers and prototypes.
6. Add functionality to detect and properly install a file into
<destbase> if the path is absolute.
7. Make the token strcpy a strlcpy just to be safe (and to be sure that
a NUL is tacked on the end).
8. Fix a bad [double-]free(3) call.
This is untested code, but will be vetted soon.
Affected files ...
.. //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#32 edit
Differences ...
==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/create/perform.c#32 (text+ko) ====
@@ -21,8 +21,8 @@
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
-/* XXX (gcooper): needs to come before sys/stat.h for stat(2). */
-#include <sys/types.h>
+
+#include <sys/types.h> /* needs to come before sys/stat.h for stat(2). */
/* Read comment below in add_file. */
#ifdef BROKEN_MMAP
#include <sys/mman.h>
@@ -234,9 +234,9 @@
}
/* Prefix should add an @cwd to the packing list */
- if (Prefix)
- if (add_plist_top(&plist, PLIST_CWD, Prefix) == -1)
- err(EXIT_FAILURE, "%s: add_plist_top failed", __func__);
+ if (add_plist_top(&plist, PLIST_CWD, (Prefix == NULL ? "." : Prefix)) ==
+ -1)
+ err(EXIT_FAILURE, "%s: add_plist_top failed", __func__);
/* Add the origin if asked, at the top */
if (Origin)
@@ -252,7 +252,7 @@
err(EXIT_FAILURE, "%s: add_plist_top failed", __func__);
if (asprintf(&cp, "PKG_FORMAT_REVISION:%d.%d", PLIST_FMT_VER_MAJOR,
- PLIST_FMT_VER_MINOR) == -1) {
+ PLIST_FMT_VER_MINOR) == -1) {
errx(2, "%s: asprintf() failed", __func__);
}
if (add_plist_top(&plist, PLIST_COMMENT, cp) == -1)
@@ -263,7 +263,7 @@
* We're just here for to dump out a revised plist for the FreeBSD ports
* hack. It's not a real create in progress.
*/
- if (PlistOnly) {
+ if (PlistOnly == TRUE) {
check_list(home, &plist);
exit(write_plist(&plist, stdout) == 0 ? 0 : 1);
}
@@ -281,82 +281,12 @@
/* copy_plist(home, &plist); */
/* mark_plist(&plist); */
- /* Now put the release specific items in */
- if (Prefix == NULL)
- if (add_plist(&plist, PLIST_CWD, ".") == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
-
- if (write_file(COMMENT_FNAME, Comment) == 0) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, COMMENT_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, COMMENT_FNAME);
- } else
+ /* Write out required files */
+ if (write_file(COMMENT_FNAME, Comment) == -1)
err(EXIT_FAILURE, "failed to write comment file");
- if (write_file(DESC_FNAME, Desc) == 0) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, DESC_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, DESC_FNAME);
- } else
+ if (write_file(DESC_FNAME, Desc) == -1)
err(EXIT_FAILURE, "failed to write description file");
- if (Install != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, INSTALL_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, INSTALL_FNAME);
- }
- if (PostInstall != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, POST_INSTALL_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, POST_INSTALL_FNAME);
- }
- if (DeInstall != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, DEINSTALL_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, DEINSTALL_FNAME);
- }
- if (PostDeInstall != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, POST_DEINSTALL_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, POST_DEINSTALL_FNAME);
- }
- if (Require != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, REQUIRE_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, REQUIRE_FNAME);
- }
- if (Display != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, DISPLAY_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, DISPLAY_FNAME);
- if (add_plist(&plist, PLIST_DISPLAY, DISPLAY_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- }
- if (Mtree != NULL) {
- if (add_plist(&plist, PLIST_IGNORE, NULL) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- if (add_plist(&plist, PLIST_FILE, MTREE_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- add_cksum(&plist, plist.tail, MTREE_FNAME);
- if (add_plist(&plist, PLIST_MTREE, MTREE_FNAME) == -1)
- err(EXIT_FAILURE, "%s: add_plist failed", __func__);
- }
-
/* Finally, write out the packing list */
fp = fopen(CONTENTS_FNAME, "w");
if (!fp)
@@ -378,7 +308,7 @@
return TRUE; /* Success */
}
-static const char*
+static const char*
add_file(struct archive *archive, const char *srcfile, const char *destfile,
const int archive_entry_open_flags)
{
@@ -469,16 +399,37 @@
}
static void
-make_dist(const char *homedir, const char *pkg, const char *suff, Package *plist)
+make_dist(const char *homedir, const char *pkg, const char *suff,
+ Package *plist)
{
-#define ADD_FILE(SRCFILE, DESTFILE, OPEN_FLAGS) \
- if (error == NULL) { \
- error = add_file(archive, SRCFILE, DESTFILE, \
+#define ADD_FILE(SRCFILE, DESTFILE, OPEN_FLAGS) do { \
+ if (error == NULL) { \
+ error = add_file(archive, SRCFILE, \
+ DESTFILE, OPEN_FLAGS); \
+ } \
+ } while (0)
+
+#define ADD_METADATA_FILE(METADATA_FILE, OPEN_FLAGS) do { \
+ ADD_FILE(METADATA_FILE, METADATA_FILE, \
OPEN_FLAGS); \
- }
+ } while (0)
+
+#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)
PackingList p;
+ char tball[PATH_MAX];
struct archive *archive = NULL;
struct lafe_matching *match_patterns = NULL;
char *destbase = NULL;
@@ -486,15 +437,16 @@
char *prefix = NULL;
char *srcbase = NULL;
char *srcfile = NULL;
- char tball[PATH_MAX];
const char *cname = NULL;
const char *error = NULL;
+
int archive_fd = -1;
int archive_open_flags;
int archive_entry_open_flags;
int archive_metadata_open_flags;
- int destbase_len, srcbase_len;
+ int destbase_len;
+ int srcbase_len;
if (*pkg == '/')
snprintf(tball, sizeof(tball), "%s.%s", pkg, suff);
@@ -577,11 +529,102 @@
}
- if (error == NULL &&
- archive_write_open_fd(archive, archive_fd) != ARCHIVE_OK)
- error = archive_error_string(archive);
+ if (error == NULL) {
+
+ if (archive_write_open_fd(archive, archive_fd) != ARCHIVE_OK)
+ error = archive_error_string(archive);
+ else if ((p = find_plist(plist, PLIST_CWD)) != NULL &&
+ p->name == NULL) {
+
+ warnx("malformed plist (first @cwd is NULL)");
+ error = strerror(EINVAL);
+
+ } else { /* first @cwd -- wewt! */
+
+ if (strlen(p->name) > PATH_MAX)
+ error = strerror(ENAMETOOLONG);
+ else {
+
+ /*
+ * We're relative to the starting prefix;
+ * dump the files here.
+ *
+ * This usecase only makes sense when
+ * installing packages directly to '/'.
+ *
+ * All metadata files will be installed into
+ * /var/db/<blah> by pkg_add .
+ */
+ if (strlen(p->name) == 1 &&
+ p->name[0] == '.' &&
+ getcwd(prefix, sizeof(prefix)) == NULL)
+ error = strerror(errno);
+ else if (strlcpy(prefix, p->name,
+ sizeof(prefix) >= sizeof(prefix)))
+ error = strerror(ENAMETOOLONG);
+
+ }
+
+ if (error == NULL) {
+
+ /* 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';
+
+ }
+
+ }
+
+ /*
+ * Add all metadata files; these don't go in the plist for
+ * the sake of simplicity.
+ */
+ ADD_METADATA_FILE(COMMENT_FNAME,
+ archive_entry_open_flags);
+
+ ADD_METADATA_FILE(DESC_FNAME,
+ archive_entry_open_flags);
+
+ if (Install != NULL) {
+ ADD_METADATA_FILE(INSTALL_FNAME,
+ archive_entry_open_flags);
+ }
+ if (PostInstall != NULL) {
+ ADD_METADATA_FILE(POST_INSTALL_FNAME,
+ archive_entry_open_flags);
+ }
+ if (DeInstall != NULL) {
+ ADD_METADATA_FILE(DEINSTALL_FNAME,
+ archive_entry_open_flags);
+ }
+ if (PostDeInstall != NULL) {
+ ADD_METADATA_FILE(POST_DEINSTALL_FNAME,
+ archive_entry_open_flags);
+ }
+ if (Require != NULL) {
+ ADD_METADATA_FILE(REQUIRE_FNAME,
+ archive_entry_open_flags);
+ }
+ if (Display != NULL) {
+ ADD_METADATA_FILE(DISPLAY_FNAME,
+ archive_entry_open_flags);
+ }
+ if (Mtree != NULL) {
+ ADD_METADATA_FILE(MTREE_FNAME,
+ archive_entry_open_flags);
+ }
+
+ }
+
+ /* Let's iterate from the first @cwd. */
+ if (p == NULL)
+ p = plist->head;
- for (p = plist->head; error == NULL && p != NULL; p = p->next)
+ for ( ; error == NULL && p != NULL; p = p->next)
switch(p->type) {
case PLIST_FILE:
@@ -593,10 +636,12 @@
destfile[0] = srcfile[0] = '\0';
/*
- * File is based off the current working directory if
- * NULL.
+ * Don't prefix with destbase if:
+ * 1. File is absolute (starts with '/').
+ * 2. destbase is NULL (file is based off the current
+ * working directory).
*/
- if (destbase != NULL)
+ if (*p->name != '/' && destbase != NULL)
if (strlcat(destfile, destbase, PATH_MAX) >
PATH_MAX)
error = strerror(ENAMETOOLONG);
@@ -629,20 +674,8 @@
/* Reset to <prefix>/<basedir> */
if (p->name == NULL) {
- /*
- * Broken plist; prefix must be defined before
- * calling `@cwd' with variable following
- * whitespace.
- */
- assert(prefix != NULL);
- /*
- * NOTE (gcooper): strcpy is safe here so long
- * as the buffers are of equal size, and also
- * because the value has been sanitized below
- * and because of the assert above.
- */
- strcpy(destbase, prefix);
+ strlcpy(destbase, prefix, sizeof(destbase));
/* Reset srcbase */
/* Tack BaseDir on the front if defined. */
@@ -662,32 +695,6 @@
*/
else {
- /* First @cwd -- wewt! */
- if (prefix == NULL) {
-
- if (strlen(p->name) > PATH_MAX)
- error = strerror(ENAMETOOLONG);
- else {
-
- prefix = p->name;
-
- /*
- * Tack BaseDir on the front if
- * defined and this is the first
- * run.
- */
- 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);
@@ -705,19 +712,6 @@
*/
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);
@@ -757,21 +751,22 @@
warnx("%s: unable to create the package '%s': %s",
__func__, tball, error);
if (0 <= archive_fd) {
- close(archive_fd);
+ (void) close(archive_fd);
if (error != NULL && unlink(tball) == -1)
warn("%s: failed to remove incomplete package - '%s'",
__func__, tball);
}
+
free(destbase);
free(destfile);
+ free(prefix);
free(srcbase);
free(srcfile);
- free(destfile);
}
static void
-sanity_check()
+sanity_check(void)
{
if (!Comment)
errx(2, "%s: required package comment string is missing (-c comment)",
@@ -817,7 +812,7 @@
return rval;
}
rc = read_plist(&plist, fd);
- close(fd);
+ (void) close(fd);
if (rc == 0) {
rval = TRUE;
More information about the p4-projects
mailing list