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

Daniel P. Berrangé berrange at redhat.com
Fri Dec 14 14:45:55 UTC 2018


On Fri, Dec 14, 2018 at 03:43:36PM +0100, Fabiano Fidêncio wrote:
> > 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" ...

No, it isn't.

The pointer in 'file' variable at this point is overwritten
with a new pointer when reading the archive entries before
we get to "cleanup".


> > 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?

Again, 'file' is used elsewhere in this method so that point is
no longer valid.

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

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 :|




More information about the Libosinfo mailing list