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

[Fabiano Fidêncio]

> Rather than checking for URI schemes, can we just call
> g_file_is_native()

Changed locally.

> 
> Also 'file' is no longer required but we've not unrefd it.

We do. We do it during the "cleanup" ...

> 
> > +        }
> > +
> > +        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


True, changed locally.

[snip]

> 
> >      if (file)
> >          g_object_unref(file);

... here!

> > +    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.

Why not taking a simpler approach and call unlink only file != NULL?

That's exactly the only case where we'd have to unlink the downloaded
file.

[snip]

I'll submit v5 with those changes and the ones required in the patch
3/3.

Thanks for the review,
-- 
Fabiano Fidêncio