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
