On Fri, Dec 07, 2018 at 02:33:47PM +0100, Fabiano Fidêncio wrote:
> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
> ---
> tools/osinfo-db-import.c | 90 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 86 insertions(+), 4 deletions(-)
>
> diff --git a/tools/osinfo-db-import.c b/tools/osinfo-db-import.c
> index 72f21ba..e4b7824 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,38 @@ 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;
> + gsize i;
> +
> + 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 (i = 0; uri_schemes[i] != NULL; i++) {
> + if (g_file_has_uri_scheme(file, uri_schemes[i])) {
> + download_file = TRUE;
> + break;
> + }
Rather than checking for URI schemes, can we just call
g_file_is_native()
Also 'file' is no longer required but we've not unrefd it.
> + }
> +
> + if (download_file) {
> + if (osinfo_db_import_download_file(source, &source_file) < 0)
This method creates a GFile for 'source' again which is a bit
of a waste of time given we already have a GFile just above
that we could pass in
> + goto cleanup;
> + } 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;
> }
>
> @@ -164,7 +244,7 @@ static int osinfo_db_import_extract(GFile *target,
> break;
> if (r != ARCHIVE_OK) {
> g_printerr("%s: cannot read next archive entry in %s: %s\n",
> - argv0, source, archive_error_string(arc));
> + argv0, source_file, archive_error_string(arc));
> goto cleanup;
> }
>
> @@ -178,7 +258,7 @@ static int osinfo_db_import_extract(GFile *target,
>
> if (archive_read_close(arc) != ARCHIVE_OK) {
> g_printerr("%s: cannot finish reading archive %s: %s\n",
> - argv0, source, archive_error_string(arc));
> + argv0, source_file, archive_error_string(arc));
> goto cleanup;
> }
>
> @@ -187,6 +267,8 @@ static int osinfo_db_import_extract(GFile *target,
> archive_read_free(arc);
> if (file)
> g_object_unref(file);
> + unlink(source_file);
This deletes the user's input file which is not nice. We must only
delete the file if it was a temp file we created.
Can we in fact call unlink immediately after archive_read_open_file()
and rely on UNIX open FD semantics.
> + g_free(source_file);
> return ret;
> }
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo
