Date: Sun, 17 Jun 2007 22:05:06 GMT From: Garrett Cooper <gcooper@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 121878 for review Message-ID: <200706172205.l5HM56Su033609@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=121878 Change 121878 by gcooper@optimus-revised_pkgtools on 2007/06/17 22:04:23 -Some improvements in permissions handling -- fchmod(fh) vs vsystem("/usr/sbin/chmod [file]"); -Use calloc(..) without temp var instead of malloc(..)/bzero(..) with temp var. -Changed "success" goto target to "finished". Affected files ... .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/add/perform.c#2 edit .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/delete/perform.c#2 edit .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/Makefile#2 edit .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/add_del.c#1 add .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/lib.h#2 edit .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/pen.c#2 edit .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/plist.c#2 edit Differences ... ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/add/perform.c#2 (text+ko) ==== @@ -201,7 +201,7 @@ /* If this is a direct extract and we didn't want it, stop now */ if (inPlace && Fake) - goto success; + goto finished; /* Finally unpack the whole mess. If extract is null we already + did so so don't bother doing it again. */ @@ -248,7 +248,7 @@ warnx("package '%s' or its older version already installed%s", Plist.name, FailOnAlreadyInstalled ? "" : " (ignored)"); code = FailOnAlreadyInstalled != FALSE; - goto success; /* close enough for government work */ + goto finished; /* close enough for government work */ } /* Now check the packing list for conflicts */ @@ -272,7 +272,7 @@ } if(conflictsfound) { if(!Force) { - warnx("please use pkg_delete first to remove conflicting package(s) or -f to force installation"); + warnx("please use pkg_delete first to remove conflicting package(s) or -f to force install"); code = 1; goto bomb; } else @@ -287,10 +287,11 @@ continue; deporigin = (p->next->type == PLIST_DEPORIGIN) ? p->next->name : NULL; if (Verbose) { - printf("Package '%s' depends on '%s'", Plist.name, p->name); - if (deporigin != NULL) - printf(" with '%s' origin", deporigin); + printf( "Package '%s' depends on '%s'", Plist.name, p->name); + if( deporigin != NULL) + printf(" with '%s' origin", deporigin); printf(".\n"); + } if (isinstalledpkg(p->name) <= 0 && !(deporigin != NULL && matchbyorigin(deporigin, NULL) != NULL)) { @@ -374,24 +375,26 @@ /* Look for the requirements file */ if (fexists(REQUIRE_FNAME)) { - vsystem("/bin/chmod +x %s", REQUIRE_FNAME); /* be sure */ - if (Verbose) - printf("Running requirements file first for %s..\n", Plist.name); - if (!Fake && vsystem("./%s %s INSTALL", REQUIRE_FNAME, Plist.name)) { - warnx("package %s fails requirements %s", pkg_fullname, - Force ? "installing anyway" : "- not installed"); - if (!Force) { - code = 1; - goto success; /* close enough for government work */ - } + +#define POST_EXEC_WARN_FORMAT_STR "package %s doesn't meet requirements %s" + + char post_exec_warn_str[ strlen(POST_EXEC_WARN_FORMAT_STR) - 2*(2) + strlen(pkg_fullname) + strlen("installing anyway") + 1 ]; + + sprintf( post_exec_warn_str, POST_EXEC_WARN_FORMAT_STR, pkg_fullname, ( Force ? "installing anyway" : "- not installed" ) ); + + if( ( code = pkg_add_del_script_chmod_and_exec( REQUIRE_FNAME, Plist, "INSTALL", 0, + "Running requirements file first for", + post_exec_warn_str ) ) + && !Force ) { + goto finished; } + } /* * Test whether to use the old method of passing tokens to installation * scripts, and set appropriate variables.. */ - if (fexists(POST_INSTALL_FNAME)) { new_m = 1; sprintf(post_script, "%s", POST_INSTALL_FNAME); @@ -407,15 +410,14 @@ /* If we're really installing, and have an installation file, run it */ if (!NoInstall && fexists(pre_script)) { - vsystem("/bin/chmod +x %s", pre_script); /* make sure */ - if (Verbose) - printf("Running pre-install for %s..\n", Plist.name); - if (!Fake && vsystem("./%s %s %s", pre_script, Plist.name, pre_arg)) { - warnx("install script returned error status"); - unlink(pre_script); - code = 1; - goto success; /* nothing to uninstall yet */ + + if( ( code = pkg_add_del_script_chmod_and_exec( pre_script, Plist, pre_arg, 1, + "Running pre-install script for", + "pre-install script exec returned non-zero status" ) + ) ) { + goto finished; } + } /* Now finally extract the entire show if we're not going direct */ @@ -423,28 +425,29 @@ extract_plist(".", &Plist); if (!Fake && fexists(MTREE_FNAME)) { - if (Verbose) - printf("Running mtree for %s..\n", Plist.name); + p = find_plist(&Plist, PLIST_CWD); - if (Verbose) - printf("mtree -U -f %s -d -e -p %s >%s\n", MTREE_FNAME, p ? p->name : "/", _PATH_DEVNULL); + + if (Verbose) { + printf( "Running mtree for %s..\nmtree -U -f %s -d -e -p %s >%s\n", + Plist.name, MTREE_FNAME, ( p ? p->name : "/" ), _PATH_DEVNULL ); + } if (!Fake) { - if (vsystem("/usr/sbin/mtree -U -f %s -d -e -p %s >%s", MTREE_FNAME, p ? p->name : "/", _PATH_DEVNULL)) + if (vsystem("/usr/sbin/mtree -U -f %s -d -e -p %s >%s", MTREE_FNAME, ( p ? p->name : "/" ), _PATH_DEVNULL)) warnx("mtree returned a non-zero status - continuing"); } } /* Run the installation script one last time? */ if (!NoInstall && fexists(post_script)) { - vsystem("/bin/chmod +x %s", post_script); /* make sure */ - if (Verbose) - printf("Running post-install for %s..\n", Plist.name); - if (!Fake && vsystem("./%s %s %s", post_script, Plist.name, post_arg)) { - warnx("install script returned error status"); - unlink(post_script); - code = 1; - goto fail; + + if( ( code = pkg_add_del_script_chmod_and_exec( post_script, Plist, post_arg, 1, + "Running post-install script for", + "post-install script exec returned non-zero status" ) + ) ) { + goto fail; } + } /* Time to record the deed? */ @@ -463,10 +466,15 @@ LogDir); bzero(LogDir, FILENAME_MAX); code = 1; - goto success; /* close enough for government work */ + goto finished; /* close enough for government work */ } /* Make sure pkg_info can read the entry */ - vsystem("/bin/chmod a+rx %s", LogDir); + int LogDir_fh = open(LogDir, O_RDONLY); + struct stat LogDir_stat; + + if(0 != fstat(LogDir_fh, &LogDir_stat)) { + fchmod(LogDir_fh, 0555); /* Ensure log dir is executable/readible to all */ + } move_file(".", DESC_FNAME, LogDir); move_file(".", COMMENT_FNAME, LogDir); if (fexists(INSTALL_FNAME)) @@ -486,9 +494,9 @@ sprintf(contents, "%s/%s", LogDir, CONTENTS_FNAME); contfile = fopen(contents, "w"); if (!contfile) { - warnx("can't open new contents file '%s'! can't register pkg", + warnx("Can't open new contents file '%s'; can't register package!", contents); - goto success; /* can't log, but still keep pkg */ + goto finished; /* can't log, but still keep pkg */ } write_plist(&Plist, contfile); fclose(contfile); @@ -502,8 +510,8 @@ NULL; if (Verbose) { printf("Trying to record dependency on package '%s'", p->name); - if (deporigin != NULL) - printf(" with '%s' origin", deporigin); + if(deporigin != NULL) + printf(" with '%s' origin ", deporigin ); printf(".\n"); } @@ -547,25 +555,23 @@ fputs(buf, stdout); putc('\n', stdout); (void) fclose(fp); - } else { - if (!Fake) { - warnx("cannot open %s as display file", buf); - } + } else if (!Fake) { + warnx("cannot open %s as display file", buf); } } - goto success; + goto finished; bomb: code = 1; - goto success; + goto finished; fail: /* Nuke the whole (installed) show, XXX but don't clean directories */ if (!Fake) delete_package(FALSE, FALSE, &Plist); - success: + finished: /* delete the packing list contents */ free_plist(&Plist); leave_playpen(); @@ -575,21 +581,19 @@ static int sanity_check(char *pkg) { - int code = 0; - if (!fexists(CONTENTS_FNAME)) { warnx("package %s has no CONTENTS file!", pkg); - code = 1; + return 1; } else if (!fexists(COMMENT_FNAME)) { warnx("package %s has no COMMENT file!", pkg); - code = 1; + return 1; } else if (!fexists(DESC_FNAME)) { warnx("package %s has no DESC file!", pkg); - code = 1; + return 1; } - return code; + return 0; } void @@ -599,11 +603,11 @@ if (!in_cleanup) { in_cleanup = 1; - if (sig) + if (sig) printf("Signal %d received, cleaning up..\n", sig); - if (!Fake && zapLogDir && LogDir[0]) + if (!Fake && zapLogDir && LogDir[0]) vsystem("%s -rf %s", REMOVE_CMD, LogDir); - leave_playpen(); + leave_playpen(); } if (sig) exit(1); ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/delete/perform.c#2 (text+ko) ==== @@ -222,15 +222,20 @@ setenv(PKG_PREFIX_VNAME, p->name, 1); if (fexists(REQUIRE_FNAME)) { - if (Verbose) - printf("Executing 'require' script.\n"); - vsystem("/bin/chmod +x %s", REQUIRE_FNAME); /* be sure */ - if (vsystem("./%s %s DEINSTALL", REQUIRE_FNAME, pkg)) { - warnx("package %s fails requirements %s", pkg, - Force ? "" : "- not deleted"); - if (!Force) - return 1; + +#define FORCE_DEL_STR "- not deleted" +#define POST_EXEC_WARN_FORMAT_STR "package %s fails requirements %s" + + /* Length of const message str + length of message str + length of 'force' string + null */ + char post_exec_warn_msg[ strlen(POST_EXEC_WARN_FORMAT_STR) - (2*2) + strlen(pkg) + strlen(FORCE_DEL_STR) + 1 ]; + + sprintf(post_exec_warn_msg, POST_EXEC_WARN_FORMAT_STR, pkg, ( Force ? "" : FORCE_DEL_STR ) ); + + if(pkg_add_del_script_chmod_and_exec(REQUIRE_FNAME, Plist, "DEINSTALL", 0, + "Executing 'require' script for", post_exec_warn_msg)) { + return 1; } + } /* @@ -253,13 +258,10 @@ if (!NoDeInstall && pre_script != NULL && fexists(pre_script)) { if (Fake) printf("Would execute de-install script at this point.\n"); - else { - vsystem("/bin/chmod +x %s", pre_script); /* make sure */ - if (vsystem("./%s %s %s", pre_script, pkg, pre_arg)) { - warnx("deinstall script returned error status"); - if (!Force) - return 1; - } + else if(pkg_add_del_script_chmod_and_exec(pre_script, Plist, pre_arg, 0, + "Executing deinstall pre-script for", "Deinstall pre-script exec returned non-zero status") + && !Force) { + return 1; } } @@ -310,13 +312,10 @@ if (!NoDeInstall && post_script != NULL && fexists(post_script)) { if (Fake) printf("Would execute post-deinstall script at this point.\n"); - else { - vsystem("/bin/chmod +x %s", post_script); /* make sure */ - if (vsystem("./%s %s %s", post_script, pkg, post_arg)) { - warnx("post-deinstall script returned error status"); - if (!Force) - return 1; - } + else if(pkg_add_del_script_chmod_and_exec(post_script, Plist, pre_arg, 0, + "Executing deinstall post-script for", "Deinstall post-script exec returned non-zero status") + && !Force) { + return 1; } } ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/Makefile#2 (text+ko) ==== @@ -3,7 +3,7 @@ LIB= install INTERNALLIB= SRCS= file.c msg.c plist.c str.c exec.c global.c pen.c match.c \ - deps.c version.c pkgwrap.c url.c + deps.c version.c pkgwrap.c url.c add_del.c WARNS?= 3 WFORMAT?= 1 ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/lib.h#2 (text+ko) ==== @@ -146,6 +146,10 @@ STAILQ_HEAD(reqr_by_head, reqr_by_entry); /* Prototypes */ + +/* _add / _delete consolidation */ +int pkg_add_del_script_chmod_and_exec(const char *, const Package, const char *, const int, const char *, const char *); + /* Misc */ int vsystem(const char *, ...); char *vpipe(const char *, ...); ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/pen.c#2 (text+ko) ==== @@ -110,8 +110,7 @@ errx(2, "%s: can't mkdir '%s'", __func__, pen); } - if (Verbose) { - if (sz) + if (Verbose && sz) { fprintf(stderr, "Requested space: %d bytes, free space: %lld bytes in %s\n", (int)sz, (long long)min_free(pen), pen); } ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/plist.c#2 (text+ko) ==== @@ -157,11 +157,7 @@ PackingList new_plist_entry(void) { - PackingList ret; - - ret = (PackingList)malloc(sizeof(struct _plist)); - bzero(ret, sizeof(struct _plist)); - return ret; + return (PackingList)calloc(1, sizeof(struct _plist)); } /* Free an entire packing list */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200706172205.l5HM56Su033609>