[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] Fix qemuMigrationToFile nonull annotation

The qemuMigrationToFile method was accidentally annotated for
the 'compressor' parameter to be non-null, instead of the
'path' parameter. Thus GCC with -O2, unhelpfully deleted the
entire 'if (compressor == NULL)' block of code during
optimization. Thus NULL was passed to virCommandNew() with
predictably bad results.

* src/qemu/qemu_migration.h: Fix non-null annotation to be
  against path instead of compressor
 src/qemu/qemu_migration.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

This shows that the 'ATTRIBUTE_NONNULL' annotation is actually
really very dangerous to use. GCC is incapable of issuing any
warnings about callers passing NULL, unless they pass a literal
"NULL". If the caller does 'void *p = NULL; foo(p)' it will not
warn. GCC is also not warning about the fact that there is a
huge block of code for 'if (compressor == NULL)' that is "dead" 
code and being deleted.

While it is perhaps nice to have ATTRIBUTE_NONNULL for static
analysis tools like clang, IMHO, it is too dangerous for us
to continue to have it enabled in builds. I think we should
define it to a no-op macro, unless explicitly enabled with

diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index f4e86c8..c0f3aa2 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -64,7 +64,7 @@ int qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm,
                         int fd, off_t offset, const char *path,
                         const char *compressor,
                         bool is_reg, bool bypassSecurityDriver)
 #endif /* __QEMU_MIGRATION_H__ */

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]