[libvirt] [libvirt-sandbox PATCH] sandbox-builder: Don't try g_file_enumerator_next_file if enumerator is NULL

Passing the NULL enumerator to g_file_enumerator_next_file makes us overwrite error information otherwise: $ sudo bin/virt-sandbox-service create --unitfile foo.service foo (process:18529): GLib-GIO-CRITICAL **: g_file_enumerator_next_file: assertion 'G_IS_FILE_ENUMERATOR (enumerator)' failed (process:18529): GLib-GIO-CRITICAL **: g_file_enumerator_close: assertion 'G_IS_FILE_ENUMERATOR (enumerator)' failed bin/virt-sandbox-service:318: Warning: GError set over the top of a previous GError or uninitialized memory. This indicates a bug in someone's code. You must ensure an error is NULL before it's set. The overwriting error message was: Error removing file: No such file or directory context.undefine() bin/virt-sandbox-service:318: Warning: g_object_unref: assertion 'G_IS_OBJECT (object)' failed context.undefine() Cleanup failed: g-io-error-quark: No such file or directory (1) bin/virt-sandbox-service: g-io-error-quark: No such file or directory (1) also make sure we don't unref NULL in this case. --- libvirt-sandbox/libvirt-sandbox-builder.c | 37 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index 1745c88..0c43b62 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -727,32 +727,35 @@ gboolean gvir_sandbox_builder_clean_post_stop(GVirSandboxBuilder *builder, errno != ENOENT) ret = FALSE; - if (!(enumerator = g_file_enumerate_children(libsFile, "*", G_FILE_QUERY_INFO_NONE, - NULL, error)) && - (*error)->code != G_IO_ERROR_NOT_FOUND) { - ret = FALSE; - goto cleanup; - } + if ((enumerator = g_file_enumerate_children(libsFile, "*", G_FILE_QUERY_INFO_NONE, + NULL, error))) { + while ((info = g_file_enumerator_next_file(enumerator, NULL, error))) { + child = g_file_enumerator_get_child(enumerator, info); + if (!g_file_delete(child, NULL, error)) + ret = FALSE; + g_object_unref(child); + child = NULL; + g_object_unref(info); + info = NULL; + } - while ((info = g_file_enumerator_next_file(enumerator, NULL, error))) { - child = g_file_enumerator_get_child(enumerator, info); - if (!g_file_delete(child, NULL, error)) + if (!g_file_enumerator_close(enumerator, NULL, error)) + ret = FALSE; + } else { + if ((*error)->code != G_IO_ERROR_NOT_FOUND) { ret = FALSE; - g_object_unref(child); - child = NULL; - g_object_unref(info); - info = NULL; + goto cleanup; + } + g_clear_error(error); } - if (!g_file_enumerator_close(enumerator, NULL, error)) - ret = FALSE; - if (!g_file_delete(libsFile, NULL, error) && (*error)->code != G_IO_ERROR_NOT_FOUND) ret = FALSE; cleanup: - g_object_unref(enumerator); + if (enumerator) + g_object_unref(enumerator); g_object_unref(libsFile); g_free(libsdir); g_free(dskfile); -- 2.7.0

On Sat, Mar 19, 2016 at 02:38:00PM +0100, Guido Günther wrote:
Passing the NULL enumerator to g_file_enumerator_next_file makes us overwrite error information otherwise:
$ sudo bin/virt-sandbox-service create --unitfile foo.service foo (process:18529): GLib-GIO-CRITICAL **: g_file_enumerator_next_file: assertion 'G_IS_FILE_ENUMERATOR (enumerator)' failed
(process:18529): GLib-GIO-CRITICAL **: g_file_enumerator_close: assertion 'G_IS_FILE_ENUMERATOR (enumerator)' failed bin/virt-sandbox-service:318: Warning: GError set over the top of a previous GError or uninitialized memory. This indicates a bug in someone's code. You must ensure an error is NULL before it's set. The overwriting error message was: Error removing file: No such file or directory context.undefine() bin/virt-sandbox-service:318: Warning: g_object_unref: assertion 'G_IS_OBJECT (object)' failed context.undefine() Cleanup failed: g-io-error-quark: No such file or directory (1) bin/virt-sandbox-service: g-io-error-quark: No such file or directory (1)
also make sure we don't unref NULL in this case. --- libvirt-sandbox/libvirt-sandbox-builder.c | 37 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 17 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Mon, Mar 21, 2016 at 09:40:35AM +0000, Daniel P. Berrange wrote:
On Sat, Mar 19, 2016 at 02:38:00PM +0100, Guido Günther wrote:
Passing the NULL enumerator to g_file_enumerator_next_file makes us overwrite error information otherwise:
$ sudo bin/virt-sandbox-service create --unitfile foo.service foo (process:18529): GLib-GIO-CRITICAL **: g_file_enumerator_next_file: assertion 'G_IS_FILE_ENUMERATOR (enumerator)' failed
(process:18529): GLib-GIO-CRITICAL **: g_file_enumerator_close: assertion 'G_IS_FILE_ENUMERATOR (enumerator)' failed bin/virt-sandbox-service:318: Warning: GError set over the top of a previous GError or uninitialized memory. This indicates a bug in someone's code. You must ensure an error is NULL before it's set. The overwriting error message was: Error removing file: No such file or directory context.undefine() bin/virt-sandbox-service:318: Warning: g_object_unref: assertion 'G_IS_OBJECT (object)' failed context.undefine() Cleanup failed: g-io-error-quark: No such file or directory (1) bin/virt-sandbox-service: g-io-error-quark: No such file or directory (1)
also make sure we don't unref NULL in this case. --- libvirt-sandbox/libvirt-sandbox-builder.c | 37 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 17 deletions(-)
ACK
Pushed. Thanks, -- Guido
participants (2)
-
Daniel P. Berrange
-
Guido Günther