[Libosinfo] [osinfo-db-tools PATCH v3 2/3] import: Learn how to deal with URLs

[Fabiano Fidêncio]

On Thu, Dec 6, 2018 at 4:04 PM Daniel P. Berrangé <berrange at redhat.com> wrote:
>
> On Thu, Dec 06, 2018 at 04:01:44PM +0100, Fabiano Fidêncio wrote:
> > Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> > ---
> >  tools/osinfo-db-import.c | 88 ++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 84 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/osinfo-db-import.c b/tools/osinfo-db-import.c
> > index 72f21ba..77ef0b9 100644
> > --- a/tools/osinfo-db-import.c
> > +++ b/tools/osinfo-db-import.c
> > @@ -134,6 +134,55 @@ static GFile *osinfo_db_import_get_file(GFile *target,
> >      return g_file_resolve_relative_path(target, tmp);
> >  }
> >
> > +static int
> > +osinfo_db_import_download_file(const gchar *url,
> > +                               gchar **source_file)
> > +{
> > +    GFile *in = NULL;
> > +    GFile *out = NULL;
> > +    GError *err = NULL;
> > +    gchar *filename = NULL;
> > +    GFileCopyFlags flags = G_FILE_COPY_OVERWRITE | G_FILE_COPY_BACKUP;
> > +    int ret = -1;
> > +
> > +    in = g_file_new_for_uri(url);
> > +    if (in == NULL)
> > +        return -1;
> > +
> > +    filename = g_file_get_basename(in);
> > +    if (filename == NULL)
> > +        goto cleanup;
> > +
> > +    *source_file = g_strdup_printf("%s/%s", g_get_user_cache_dir(), filename);
> > +    if (*source_file == NULL)
> > +        goto cleanup;
> > +
> > +    out = g_file_new_for_path(*source_file);
> > +    if (out == NULL)
> > +        goto cleanup;
> > +
> > +    if (!g_file_copy(in, out, flags, NULL, NULL, NULL, &err)) {
> > +        g_printerr("Could not download file \"%s\": %s\n",
> > +                   url, err->message);
> > +        goto cleanup;
> > +    }
> > +
> > +    ret = 0;
> > +
> > + cleanup:
> > +    g_free(filename);
> > +    if (in != NULL)
> > +        g_object_unref(in);
> > +    if (out != NULL)
> > +        g_object_unref(out);
> > +    if (err != NULL)
> > +        g_error_free(err);
> > +    if (ret != 0)
> > +        unlink(*source_file);
> > +
> > +    return ret;
> > +}
> > +
> >  static int osinfo_db_import_extract(GFile *target,
> >                                      const char *source,
> >                                      gboolean verbose)
> > @@ -143,6 +192,8 @@ static int osinfo_db_import_extract(GFile *target,
> >      int ret = -1;
> >      int r;
> >      GFile *file = NULL;
> > +    gchar *source_file = NULL;
> > +    const gchar *uri_schemes[] = {"ftp", "http", NULL};
> >
> >      arc = archive_read_new();
> >
> > @@ -152,9 +203,37 @@ static int osinfo_db_import_extract(GFile *target,
> >      if (source != NULL && g_str_equal(source, "-"))
> >          source = NULL;
> >
> > -    if ((r = archive_read_open_filename(arc, source, 10240)) != ARCHIVE_OK) {
> > +    if (source != NULL) {
> > +        gboolean download_file = FALSE;
> > +
> > +        file = g_file_new_for_uri(source);
> > +        if (file == NULL)
> > +            goto cleanup;
> > +
> > +        /* The supported uri schemes here are "ftp", "http" and "https".
> > +         * However, "https" is not set as part of the array as it matches with
> > +         * "http". */
> > +        for (gint i = 0; uri_schemes[i] != NULL; i++) {
>
> s/gint/gsize/ for array indexes

Which also reminds me to avoid declarations inside the for loop.

>
> > +            if (g_file_has_uri_scheme(file, uri_schemes[i])) {
> > +                download_file = TRUE;
> > +                break;
> > +            }
> > +        }
> > +
> > +        if (download_file) {
> > +            if (osinfo_db_import_download_file(source, &source_file) < 0)
> > +                goto cleanup;
>
> Something needs to delete the file that this call downloads locally....
>
> > +        } else {
> > +            source_file = g_strdup(source);
> > +        }
> > +
> > +        if (source_file == NULL)
> > +            goto cleanup;
> > +    }
> > +
> > +    if ((r = archive_read_open_filename(arc, source_file, 10240)) != ARCHIVE_OK) {
> >          g_printerr("%s: cannot open archive %s: %s\n",
> > -                   argv0, source, archive_error_string(arc));
> > +                   argv0, source_file, archive_error_string(arc));
> >          goto cleanup;
> >      }
> >
>
> > @@ -187,6 +266,7 @@ static int osinfo_db_import_extract(GFile *target,
> >      archive_read_free(arc);
> >      if (file)
> >          g_object_unref(file);
> > +    g_free(source_file);
>
> ie unlink before here
>
> >      return ret;
> >  }
>

Do you want a v4 because of those changes or can I go ahead with the
first 2 patches after doing the fixes locally?