Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 25 Jul 2010 04:09:33 -0700
From:      Garrett Cooper <gcooper@FreeBSD.org>
To:        Julien Laffaye <jlaffaye@freebsd.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 181439 for review
Message-ID:  <AANLkTi=ZTV=tyFod0efy2O-Du2rprzkYz87JMfYW0V_F@mail.gmail.com>
In-Reply-To: <201007242051.o6OKpF5t030159@repoman.freebsd.org>
References:  <201007242051.o6OKpF5t030159@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help 
On Sat, Jul 24, 2010 at 1:51 PM, Julien Laffaye <jlaffaye@freebsd.org> wrot=
e:
> http://p4web.freebsd.org/@@181439?ac=3D10
>
> Change 181439 by jlaffaye@jlaffaye-chulak on 2010/07/24 20:51:13
>
> =A0 =A0 =A0 =A0Add support for fetching packages. Read them on the fly th=
anks
> =A0 =A0 =A0 =A0to libarchive(3) callbacks.
> =A0 =A0 =A0 =A0Fix segfault by not plist_free'ing an unitialised plist.

Was this perhaps a problem with the structure not being NULL'ed out
after it was free'd?

> Affected files ...
>
> .. //depot/projects/soc2010/pkg_complete/lib/libpkg/pkg.h#7 edit
> .. //depot/projects/soc2010/pkg_complete/lib/libpkg/url.c#3 edit
> .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/extract=
.c#6 edit
> .. //depot/projects/soc2010/pkg_complete/usr.sbin/pkg_install/add/perform=
.c#6 edit
>
> Differences ...
>
> =3D=3D=3D=3D //depot/projects/soc2010/pkg_complete/lib/libpkg/pkg.h#7 (te=
xt+ko) =3D=3D=3D=3D
>
> @@ -33,6 +33,7 @@
> =A0#include <sys/stat.h>
> =A0#include <sys/queue.h>
> =A0#include <sys/utsname.h>
> +#include <archive.h>

This should be moved down below all of the standard system headers
(libarchive might require some of the headers above.. fetch.h has
similar requirements)...

> =A0#include <ctype.h>
> =A0#include <dirent.h>
> =A0#include <stdarg.h>
> @@ -144,6 +145,12 @@
> =A0};
> =A0STAILQ_HEAD(reqr_by_head, reqr_by_entry);
>
> +struct fetch_data {
> + =A0 =A0 =A0 FILE *ftp;
> + =A0 =A0 =A0 int pkgfd;
> + =A0 =A0 =A0 char buf[8192];
> +};

Using BUFSIZ might be a better idea.

> +
> =A0/* Prototypes */
> =A0/* Misc */
> =A0int =A0 =A0 =A0 =A0 =A0 =A0vsystem(const char *, ...);
> @@ -173,6 +180,8 @@
> =A0Boolean =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0issymlink(const char *);
> =A0Boolean =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0isURL(const char *);
> =A0const char =A0 =A0 *fileGetURL(const char *, const char *, int);
> +int =A0 =A0 =A0 =A0 =A0 =A0fetch_archive(struct archive *, const char *,=
 const char *,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Boolean);
> =A0char =A0 =A0 =A0 =A0 =A0 *fileFindByPath(const char *, const char *);
> =A0char =A0 =A0 =A0 =A0 =A0 *fileGetContents(const char *);
> =A0ssize_t =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0write_file(const char *, const =
char *);
>
> =3D=3D=3D=3D //depot/projects/soc2010/pkg_complete/lib/libpkg/url.c#3 (te=
xt+ko) =3D=3D=3D=3D
>
> @@ -23,15 +23,23 @@
>
> =A0#include <sys/param.h>
> =A0#include <sys/wait.h>
> +#include <archive.h>
> =A0#include <err.h>
> +#include <errno.h>
> =A0#include <libgen.h>
> =A0#include <stdio.h>
> =A0#include <fetch.h> =A0 =A0 /* NOTE: stdio must come before fetch. */
> =A0#include "pkg.h"
>
> +static ssize_t archive_read_cb(struct archive *, void *, const void **);
> +static int archive_open_cb(struct archive *a, void *);
> +static int archive_close_cb(struct archive *, void *);
> +
> =A0/*
> =A0* Try and fetch a file by URL, returning the directory name for where
> =A0* it's unpacked, if successful.
> + * XXX (jlaffaye): to be removed when all call to fileGetURL() are conve=
rted to
> + * fetch_archive()
> =A0*/
> =A0const char *
> =A0fileGetURL(const char *base, const char *spec, int keep_package)
> @@ -113,11 +121,11 @@
> =A0 =A0 =A0 =A0fetchDebug =3D (Verbose > 0);
> =A0 =A0 =A0 =A0if ((ftp =3D fetchGetURL(fname, Verbose ? "v" : NULL)) =3D=
=3D NULL) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("Error: Unable to get %s: %s\n", fna=
me,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 fetchLastErrString);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 fetchLastErrString);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* If the fetch fails, yank the package. *=
/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (keep_package && unlink(pkg) < 0) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0warnx("failed to remove pa=
rtially fetched package: %s",
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pkg);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pkg);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (NULL);
> =A0 =A0 =A0 =A0}
> @@ -182,3 +190,170 @@
> =A0 =A0 =A0 =A0return (rp);
>
> =A0}
> +
> +/*
> + * Setup the archive `a' callbacks to read data from an URL `spec' via f=
etch(3).
> + * If `spec' is not an URL, the function try to find the location of the=
 file
> + * via `base' or via the environment variable `PKG_ADD_BASE'.
> + * Returns 0 on success, 1 otherwise.
> + */
> +int
> +fetch_archive(struct archive *a, const char *base, const char *spec,
> + =A0 =A0 =A0 =A0 =A0 =A0 Boolean keep_package)
> +{
> + =A0 =A0 =A0 struct fetch_data *data =3D NULL;
> + =A0 =A0 =A0 char *cp, *hint, *tmp;
> + =A0 =A0 =A0 char fname[FILENAME_MAX];
> + =A0 =A0 =A0 char pkg[FILENAME_MAX];
> + =A0 =A0 =A0 int retcode =3D 0;
> +
> + =A0 =A0 =A0 if ((data =3D malloc(sizeof(struct fetch_data))) =3D=3D NUL=
L)
> + =A0 =A0 =A0 =A0 =A0 err(EXIT_FAILURE, "malloc()");
> +
> + =A0 =A0 =A0 if (!isURL(spec)) {
> + =A0 =A0 =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0 =A0 =A0* We've been given an existing URL (that's known=
-good) and now
> + =A0 =A0 =A0 =A0 =A0 =A0* we need to construct a composite one out of th=
at and the
> + =A0 =A0 =A0 =A0 =A0 =A0* basename we were handed as a dependency.
> + =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 if (base !=3D NULL) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcpy(fname, base);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Advance back two slashes to get to the=
 root of the
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* package hierarchy
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 cp =3D strrchr(fname, '/');
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (cp) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *cp =3D '\0'; /* chop name */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cp =3D strrchr(fname, '/');
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (cp !=3D NULL) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 *(cp + 1) =3D '\0';
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcat(cp, "All/");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcat(cp, spec);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcat(cp, ".tbz");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 retcode =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto cleanup;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 /* Special tip that sysinstall left for us */
> + =A0 =A0 =A0 =A0 =A0 else if ((hint =3D getenv("PKG_ADD_BASE")) !=3D NUL=
L) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* Otherwise, we've been given an environ=
ment variable
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* hinting at the right location from sys=
install
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcpy(fname, hint);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcat(fname, spec);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 strcat(fname, ".tbz");
> + =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 /* We dont have an url and are unable to guess one =
*/
> + =A0 =A0 =A0 =A0 =A0 else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 retcode =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto cleanup;
> + =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 strcpy(fname, spec);
> +
> + =A0 =A0 =A0 if (keep_package) {
> + =A0 =A0 =A0 =A0 =A0 tmp =3D getenv("PKGDIR");
> + =A0 =A0 =A0 =A0 =A0 strlcpy(pkg, tmp ? tmp : ".", sizeof(pkg));
> + =A0 =A0 =A0 =A0 =A0 tmp =3D basename(fname);
> + =A0 =A0 =A0 =A0 =A0 strlcat(pkg, "/", sizeof(pkg));
> + =A0 =A0 =A0 =A0 =A0 strlcat(pkg, tmp, sizeof(pkg));
> +
> + =A0 =A0 =A0 =A0 =A0 data->pkgfd =3D open(pkg, O_WRONLY|O_CREAT|O_TRUNC,=
 0644);
> + =A0 =A0 =A0 =A0 =A0 if (data->pkgfd =3D=3D -1) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 warn("Error: Unable to open %s", pkg);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 retcode =3D 1;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto cleanup;
> + =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 } else
> + =A0 =A0 =A0 =A0 =A0 data->pkgfd =3D 0;
> +
> + =A0 =A0 =A0 fetchDebug =3D (Verbose > 0);
> + =A0 =A0 =A0 if ((data->ftp =3D fetchGetURL(fname, Verbose ? "v" : NULL)=
) =3D=3D NULL) {
> + =A0 =A0 =A0 =A0 =A0 warnx("Error: Unable to get %s: %s\n", fname, fetch=
LastErrString);
> + =A0 =A0 =A0 =A0 =A0 /* If the fetch fails, yank the package. */
> + =A0 =A0 =A0 =A0 =A0 if (keep_package && unlink(pkg) < 0)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 warnx("failed to remove partially fetched p=
ackage: %s", pkg);
> + =A0 =A0 =A0 =A0 =A0 retcode =3D 1;
> + =A0 =A0 =A0 =A0 =A0 goto cleanup;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if (isatty(0) || Verbose) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("Fetching %s...", fname);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 fflush(stdout);
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if (archive_read_open(a, data, archive_open_cb, archive_rea=
d_cb,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 archive_close_c=
b) !=3D ARCHIVE_OK) {
> + =A0 =A0 =A0 =A0 =A0 warnx("Can not open '%s': %s", pkg, archive_error_s=
tring(a));
> + =A0 =A0 =A0 =A0 =A0 retcode =3D 1;
> + =A0 =A0 =A0 =A0 =A0 goto cleanup;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 cleanup:
> + =A0 =A0 =A0 if (retcode =3D=3D 1 && data !=3D NULL)
> + =A0 =A0 =A0 =A0 =A0 free(data);
> +
> + =A0 =A0 =A0 return (retcode);
> +}
> +
> +/*
> + * Libarchive callback called when more data is needed.
> + * Read the data from the fetch(3) file descriptor and store it into buf=

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=ZTV=tyFod0efy2O-Du2rprzkYz87JMfYW0V_F>