After short discussion on IRC, I pushed this as "trivial".
On 11/13/20 12:42 PM, Laine Stump wrote:
Before commit 24d8968c, virDirClose took a DIR**, and that was never
NULL, so its declaration included ATTRIBUTE_NONNULL(1). Since that
commit, virDirClose takes a DIR*, and it may be NULL (e.g. if the DIR*
is initialized to NULL and was never closed).
Even though virDirClose() is currently only called implicitly (as the
cleanup for a g_autoptr(DIR)), and (as I've just newly learned) the
autocleanup function g_autoptr will only be called if the pointer in
question is non-null (see the definition of
_GLIB_AUTOPTR_CLEAR_FUNC_NAME in
/usr/include/glib-2.0/glib/gmacros.h), it does still cause Coverity to
complain that it *could* be called with a NULL, and it's also possible
that in the future someone might add code that explicitly calls
virDirClose.
To eliminate the Coverity complaints, and protect against the
hypothetical future where someone both explicitly calls virDirClose()
with a potentially NULL value, *and* re-enables the nonnull directive
when not building with Coverity (disabled by commit eefb881) this
patch removes the ATTRIBUTE_NONNULL(1) from the declaration of
virDirClose().
Fixes: 24d8968cd0a718af4badbbc858b1b449fea7205a
Details-Research-by: Dan Berrange <berrange@xxxxxxxxxx>
Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
---
src/util/virfile.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virfile.h b/src/util/virfile.h
index ba31a78e58..32505826ee 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -269,8 +269,7 @@ int virDirOpenQuiet(DIR **dirp, const char *dirname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
-void virDirClose(DIR *dirp)
- ATTRIBUTE_NONNULL(1);
+void virDirClose(DIR *dirp);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(DIR, virDirClose);
int virFileMakePath(const char *path) G_GNUC_WARN_UNUSED_RESULT;
