Date: Sat, 19 Jun 2010 00:31:29 GMT From: Julien Laffaye <jlaffaye@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 179817 for review Message-ID: <201006190031.o5J0VT62010908@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@179817?ac=10 Change 179817 by jlaffaye@jlaffaye-chulak on 2010/06/19 00:30:38 Errors checking, comments, ... Affected files ... .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/complete/main.c#3 edit .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/complete/perform.c#4 edit Differences ... ==== //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/complete/main.c#3 (text+ko) ==== @@ -28,6 +28,10 @@ if ((dir = dirname(argv[1])) == NULL) err(1, "dirname(%s)", argv[1]); + /* + * Take the real path of the target file + * because we are going to chdir() + */ if ((realpath(argv[2], out)) == NULL) err(1, "realpath(%s)", argv[2]); @@ -40,6 +44,6 @@ static void usage(void) { - fprintf(stderr, "usage: pkg_complete source_package target_package\n"); - exit(1); + fprintf(stderr, "usage: pkg_complete source-package target-package\n"); + exit(1); } ==== //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/complete/perform.c#4 (text+ko) ==== @@ -22,23 +22,31 @@ struct stat st; struct list_deps deps; char fname[PATH_MAX], buf[1024], *ext; - size_t r; + ssize_t r; int fd, err = 0; - if ((ext = strrchr(pkgname, '.')) == NULL) + if ((ext = strrchr(pkgname, '.')) == NULL) { warn("strrchr()"); + return (1); + } deps.size = 100; deps.len = 0; deps.pkgs = malloc(sizeof(Package)*(deps.size)); - pkg_get_deps(pkgname, ext, &deps); + + if(pkg_get_deps(pkgname, ext, &deps) > 0) + return (1); qsort(deps.pkgs, deps.len, sizeof(Package), pkg_sort_deps); a = archive_write_new(); archive_write_set_format_ustar(a); archive_write_set_compression_none(a); - archive_write_open_filename(a, outname); + + if (archive_write_open_filename(a, outname) != ARCHIVE_OK) { + warnx("%s", archive_error_string(a)); + return (1); + } entry = archive_entry_new(); @@ -54,9 +62,8 @@ archive_entry_clear(entry); for (size_t i = 0; i < deps.len; i++) { - snprintf(fname, sizeof(fname), "%s%s", - deps.pkgs[i].name, ext); - if((fd = open(fname, O_RDONLY)) < 0) { + snprintf(fname, sizeof(fname), "%s%s", deps.pkgs[i].name, ext); + if ((fd = open(fname, O_RDONLY)) < 0) { warn("open(%s)", fname); err = 1; break; @@ -66,10 +73,18 @@ archive_entry_set_pathname(entry, fname); archive_write_header(a, entry); for (;;) { - r = read(fd, buf, sizeof(buf)); + if ((r = read(fd, buf, sizeof(buf))) == -1) { + warn("read()"); + err = 1; + break; + } if (r <= 0) break; - archive_write_data(a, buf, r); + if (archive_write_data(a, buf, r) != r) { + warnx("%s", archive_error_string(a)); + err = 1; + break; + } } close(fd); archive_entry_clear(entry); @@ -80,9 +95,25 @@ for (size_t i = 0; i < deps.len; i++) free_plist(&deps.pkgs[i]); free(deps.pkgs); + + /* + * An error at this level means that we failed to create the archive. + * Be nice and remove this useless file. + */ + if (err != 0) { + warnx("removing corrupted package file"); + unlink(outname); + } + return (err); } +/* + * Get the dependencies of the `pkgname` package file, storing them in the + * `deps->pkgs` array. The function calls itself recursiveely for each + * dependency of `pkgname`. + * Retuns a non zero value on failure. + */ static int pkg_get_deps(char *pkgname, char *ext, struct list_deps *deps) { @@ -97,48 +128,52 @@ short found = 0; if ((plist_size = unpack_to_buffer(pkgname, CONTENTS_FNAME, &plist_buf)) - == -1) { - warn("unpack_to_buffer()"); - err = 1; - } else { - pkg.head = pkg.tail = NULL; - read_plist_from_buffer(&pkg, plist_buf, plist_size); - free(plist_buf); + == -1) { + warnx("error while unpacking %s", CONTENTS_FNAME); + return (1); + } + + pkg.head = pkg.tail = NULL; + read_plist_from_buffer(&pkg, plist_buf, plist_size); + free(plist_buf); - /* Register the current package */ - if (deps->size <= deps->len) { - deps->size *= 2; - deps->pkgs = realloc(deps->pkgs, - sizeof(Package)*(deps->size)); - } - deps->pkgs[deps->len] = pkg; - deps->len++; + /* Register the current package */ + if (deps->size <= deps->len) { + deps->size *= 2; + deps->pkgs = realloc(deps->pkgs, sizeof(Package)*(deps->size)); + } + deps->pkgs[deps->len] = pkg; + deps->len++; - /* - * Get the dependencies of pkgname's dependencies, - * if not already done. - */ - for (p = pkg.head; p; p = p->next) { - if(p->type == PLIST_PKGDEP) { - found = 0; - for (i=0; i < deps->len; i++) { - if (strcmp(deps->pkgs[i].name, - p->name) == 0) { - found = 1; - break; - } + /* + * Get the dependencies of pkgname's dependencies, + * if not already done. + */ + for (p = pkg.head; p; p = p->next) { + if (p->type == PLIST_PKGDEP) { + found = 0; + for (i=0; i < deps->len; i++) { + if (strcmp(deps->pkgs[i].name, p->name) == 0) { + found = 1; + break; } - if (found == 0) { - snprintf(fname, sizeof(fname), - "%s%s", p->name, ext); - err += pkg_get_deps(fname, ext, deps); - } + } + if (found == 0) { + snprintf(fname, sizeof(fname), "%s%s", p->name, + ext); + err += pkg_get_deps(fname, ext, deps); } } } + return (err); } +/* + * Callback for qsort(3) + * Returns 1 if the package `a` depends on `b`, -1 if `a` is a dependency + * of `b`, or 0 if the packages are not related. + */ static int pkg_sort_deps(const void *a, const void *b) { @@ -154,7 +189,7 @@ return (0); } -/* Returns 1 if pkga depends on pkgb, 0 otherwise */ +/* Returns 1 if `pkga` depends on `pkgb`, 0 otherwise */ static int pkg_depend_on(const Package *pkga, const Package *pkgb) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201006190031.o5J0VT62010908>