On Fri, Apr 13, 2012 at 11:12:54AM +0200, Michal Privoznik wrote:
> Since compilers are trying to optimize code they are allowed to
> reorder evaluation of conditions in if statement (okay, not in all
> cases, but they can in this one). Therefore if we do:
> if (stat(file, &st) == 0 && unlink(file) < 0)
> after compiler chews this it may get feeling that swapping order
> is a good idea. However, we obviously don't want to call stat()
> on just unlink()-ed file.
Really ? I'm not sure I believe that. IIRC in-order short-circuit
evaluation is a part of the C standard. Compilers can't do any
optimization which changes the order of evalation without breaking
countless C programs.
> ---
> src/qemu/qemu_driver.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 65ed290..037d45c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9998,9 +9998,14 @@ qemuDomainSnapshotUndoSingleDiskActive(struct qemud_driver *driver,
> VIR_WARN("Unable to restore security label on %s", disk->src);
> if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
> VIR_WARN("Unable to release lock on %s", disk->src);
> +
> + /* Deliberately do not join these two ifs. Compiler may mix up
> + * the order of evaluation so unlink() may proceed stat()
> + * which is not what we want */
> if (need_unlink && stat(disk->src, &st) == 0 &&
> - st.st_size == 0 && S_ISREG(st.st_mode) && unlink(disk->src) < 0)
> - VIR_WARN("Unable to remove just-created %s", disk->src);
> + st.st_size == 0 && S_ISREG(st.st_mode))
> + if (unlink(disk->src) < 0)
> + VIR_WARN("Unable to remove just-created %s", disk->src);
>
> /* Update vm in place to match changes. */
> VIR_FREE(disk->src);
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list