Re: [PATCH] util: storage: Fix build after 25924dec0f9329d429aadae14e273602307e2214

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

 



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


Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]