On 07/09/14 10:12, Michal Privoznik wrote:
On 09.07.2014 10:05, Peter Krempa wrote:
The commit referenced above changed function arguments of virStorageFileGetMetadataFromBuf() but didn't tweak the ATTRIBUTE_NONNULL tied to them. This was caught by coverity as it actually obeys them. We disabled them for GCC and thus it didn't show up.
Additionally in commit 3ea661deeabadc3c114dfb6f662b9fd17d714a01 I passed NULL to the backingFormat argument which was also marked as nonnull. Use a dummy int's address when the argument isn't supplied so that the code doesn't need to change much. --- src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 744a6ba..528cfad 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -301,8 +301,7 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, size_t len, int format, int *backingFormat) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) - ATTRIBUTE_NONNULL(5); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virStorageFileChainGetBroken(virStorageSourcePtr chain, char **broken_file);
So if we need to cripple passing NULL, why do we even have ATTRIBUTE_NONNULL? Wouldn't it be simpler if virStorageFileGetMetadataFromBuf would accept NULL?
This patch actually removes the ATTRIBUTE_NONNULL from two places. Argument 4, as it's now an int and the attribute doesn't make sense there. And argument 5 as we are passing NULL in one place now. Therefore I also added the null tolerance code as the underlying code doesn't tolerate NULL. In virStorageFileGetMetadataFromFD we do the same stuff. Peter