Re: [PATCH 0/3] batch: don't require checking retvalue of some bitmap ops

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

 




On 30.07.2020 18:49, Nikolay Shirokovskiy wrote:
> 
> 
> On 30.07.2020 17:56, Peter Krempa wrote:
>> On Thu, Jul 30, 2020 at 17:27:35 +0300, Nikolay Shirokovskiy wrote:
>>> Most of bitmap setBit/clearBit/getBit users know that the bitmap index is
>>> not out of bound and thus don't check the return value. More precisely
>>> the stats is next:
>>>
>>> Method                all   check
>>> ===================================
>>> virBitmapSetBit        85      14
>>> virBitmapClearBit      15       3
>>> virBitmapGetBit        15       6
>>>
>>> where 'all' is the number of all occurences of the method and 'check' is the
>>> number of occurences with 'if (method' pattern.
>>>
>>> Thus keeping the retvalue checking requirement produces more
>>> noise then helps. I guess we even can make these function return
>>> void as users can simply compare the index with the bitmap size.
>>
>> Well. An ignore_value is not really expensive and it makes the callers
>> aware that something needs to be checked.
> 
> The only condition these methods can fail is out of bound. Most of
> time it is known by the caller that there is no out of bound condition.
> So when compiler suggests to check error I personally go and read
> the code only to found it can not fail in my circumstances.
> At the same time it is quite obvious that these function can not
> produce something meaningful for out of bound. That's why
> I even thinking of why don't make these methods return void. 
> 
>>
>> I don't really see the point of this.
>>
>> Additionally, individual patches are missing justification in the commit
>> message. Mentioning it in the cover letter is not enough as that doesn't
>> get comitted.
>>
> 
> I thought that writing same justification 3 times in a row will be
> too much. At the same time writing some short version will not explain
> things thoroughly. May be I should add good explanation only
> to the first patch.
> 

Hi, everyone.

Is there other opinions on the topic or I can forget about the series and
let it go?)

Nikolay




[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]

  Powered by Linux