PERFORCE change 181439 for review

[Garrett Cooper]

On Sat, Jul 24, 2010 at 1:51 PM, Julien Laffaye <jlaffaye at freebsd.org> wrote:
> http://p4web.freebsd.org/@@181439?ac=10
>
> Change 181439 by jlaffaye at jlaffaye-chulak on 2010/07/24 20:51:13
>
>        Add support for fetching packages. Read them on the fly thanks
>        to libarchive(3) callbacks.
>        Fix 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 ...
>
> ==== //depot/projects/soc2010/pkg_complete/lib/libpkg/pkg.h#7 (text+ko) ====
>
> @@ -33,6 +33,7 @@
>  #include <sys/stat.h>
>  #include <sys/queue.h>
>  #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)...

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

Using BUFSIZ might be a better idea.

> +
>  /* Prototypes */
>  /* Misc */
>  int            vsystem(const char *, ...);
> @@ -173,6 +180,8 @@
>  Boolean                issymlink(const char *);
>  Boolean                isURL(const char *);
>  const char     *fileGetURL(const char *, const char *, int);
> +int            fetch_archive(struct archive *, const char *, const char *,
> +                             Boolean);
>  char           *fileFindByPath(const char *, const char *);
>  char           *fileGetContents(const char *);
>  ssize_t                write_file(const char *, const char *);
>
> ==== //depot/projects/soc2010/pkg_complete/lib/libpkg/url.c#3 (text+ko) ====
>
> @@ -23,15 +23,23 @@
>
>  #include <sys/param.h>
>  #include <sys/wait.h>
> +#include <archive.h>
>  #include <err.h>
> +#include <errno.h>
>  #include <libgen.h>
>  #include <stdio.h>
>  #include <fetch.h>     /* NOTE: stdio must come before fetch. */
>  #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 *);
> +
>  /*
>  * Try and fetch a file by URL, returning the directory name for where
>  * it's unpacked, if successful.
> + * XXX (jlaffaye): to be removed when all call to fileGetURL() are converted to
> + * fetch_archive()
>  */
>  const char *
>  fileGetURL(const char *base, const char *spec, int keep_package)
> @@ -113,11 +121,11 @@
>        fetchDebug = (Verbose > 0);
>        if ((ftp = fetchGetURL(fname, Verbose ? "v" : NULL)) == NULL) {
>                warnx("Error: Unable to get %s: %s\n", fname,
> -                   fetchLastErrString);
> +                     fetchLastErrString);
>                /* If the fetch fails, yank the package. */
>                if (keep_package && unlink(pkg) < 0) {
>                        warnx("failed to remove partially fetched package: %s",
> -                           pkg);
> +                             pkg);
>                }
>                return (NULL);
>        }
> @@ -182,3 +190,170 @@
>        return (rp);
>
>  }
> +
> +/*
> + * Setup the archive `a' callbacks to read data from an URL `spec' via fetch(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,
> +             Boolean keep_package)
> +{
> +       struct fetch_data *data = NULL;
> +       char *cp, *hint, *tmp;
> +       char fname[FILENAME_MAX];
> +       char pkg[FILENAME_MAX];
> +       int retcode = 0;
> +
> +       if ((data = malloc(sizeof(struct fetch_data))) == NULL)
> +           err(EXIT_FAILURE, "malloc()");
> +
> +       if (!isURL(spec)) {
> +           /*
> +            * We've been given an existing URL (that's known-good) and now
> +            * we need to construct a composite one out of that and the
> +            * basename we were handed as a dependency.
> +            */
> +           if (base != NULL) {
> +               strcpy(fname, base);
> +               /*
> +                * Advance back two slashes to get to the root of the
> +                * package hierarchy
> +                */
> +               cp = strrchr(fname, '/');
> +               if (cp) {
> +                   *cp = '\0'; /* chop name */
> +                   cp = strrchr(fname, '/');
> +               }
> +               if (cp != NULL) {
> +                   *(cp + 1) = '\0';
> +                   strcat(cp, "All/");
> +                   strcat(cp, spec);
> +                   strcat(cp, ".tbz");
> +               } else {
> +                   retcode = 1;
> +                   goto cleanup;
> +               }
> +           }
> +           /* Special tip that sysinstall left for us */
> +           else if ((hint = getenv("PKG_ADD_BASE")) != NULL) {
> +               /*
> +                * Otherwise, we've been given an environment variable
> +                * hinting at the right location from sysinstall
> +                */
> +               strcpy(fname, hint);
> +               strcat(fname, spec);
> +               strcat(fname, ".tbz");
> +           }
> +           /* We dont have an url and are unable to guess one */
> +           else {
> +               retcode = 1;
> +               goto cleanup;
> +           }
> +       }
> +       else
> +           strcpy(fname, spec);
> +
> +       if (keep_package) {
> +           tmp = getenv("PKGDIR");
> +           strlcpy(pkg, tmp ? tmp : ".", sizeof(pkg));
> +           tmp = basename(fname);
> +           strlcat(pkg, "/", sizeof(pkg));
> +           strlcat(pkg, tmp, sizeof(pkg));
> +
> +           data->pkgfd = open(pkg, O_WRONLY|O_CREAT|O_TRUNC, 0644);
> +           if (data->pkgfd == -1) {
> +               warn("Error: Unable to open %s", pkg);
> +               retcode = 1;
> +               goto cleanup;
> +           }
> +       } else
> +           data->pkgfd = 0;
> +
> +       fetchDebug = (Verbose > 0);
> +       if ((data->ftp = fetchGetURL(fname, Verbose ? "v" : NULL)) == NULL) {
> +           warnx("Error: Unable to get %s: %s\n", fname, fetchLastErrString);
> +           /* If the fetch fails, yank the package. */
> +           if (keep_package && unlink(pkg) < 0)
> +               warnx("failed to remove partially fetched package: %s", pkg);
> +           retcode = 1;
> +           goto cleanup;
> +       }
> +
> +       if (isatty(0) || Verbose) {
> +               printf("Fetching %s...", fname);
> +               fflush(stdout);
> +       }
> +
> +       if (archive_read_open(a, data, archive_open_cb, archive_read_cb,
> +                             archive_close_cb) != ARCHIVE_OK) {
> +           warnx("Can not open '%s': %s", pkg, archive_error_string(a));
> +           retcode = 1;
> +           goto cleanup;
> +       }
> +
> +       cleanup:
> +       if (retcode == 1 && data != NULL)
> +           free(data);
> +
> +       return (retcode);
> +}
> +
> +/*
> + * Libarchive callback called when more data is needed.
> + * Read the data from the fetch(3) file descriptor and store it into buf