Libguestfs - Nov 2021 - [PATCH nbdkit 2/2] common/include/checked-overflow.h: Provide fallback

On 11/22/21 23:31, Richard W.M. Jones wrote:
> 
> (Catching up ...)
Welcome back! :)
> The maths looked reasonable.
Thanks!
> Did you have a version of the patch for review?
No, not yet. Wanted to clear the style questions on the new code (the
additions) at first. Next, I'll have to work those additions (the
fallback code) into your original patch -- I'll steal the way you check
for the necessity of the fallback etc.

(Meanwhile I've worked on RHBZ#1931821, with the realization that the
"dosfstools" change was inexcusable, and either way, we need
"parted" to
learn dealing with the bogus partition table.)
> My only other thought is that a simple set of tests could be good.
> However it's not worth having tests that only test if __builtin*
> functions are correct (hopefully GCC is already testing that).  So
> tests would have to check the fallback macros are correct, even if
> they are not used on the current platform.
So: the fallbacks need to be available (= built) in the source code
unconditionally, so they can be directly called by the test suite. The
actual "falling back" to them must be separate. Is that what you mean?

Thanks
Laszlo

On 11/23/21 11:44, Laszlo Ersek wrote:
> On 11/22/21 23:31, Richard W.M. Jones wrote:
>>
>> (Catching up ...)
> 
> Welcome back! :)
> 
>> The maths looked reasonable.
> 
> Thanks!
> 
>> Did you have a version of the patch for review?
> 
> No, not yet. Wanted to clear the style questions on the new code (the
> additions) at first. Next, I'll have to work those additions (the
> fallback code) into your original patch -- I'll steal the way you check
> for the necessity of the fallback etc.
> 
> (Meanwhile I've worked on RHBZ#1931821, with the realization that the
> "dosfstools" change was inexcusable, and either way, we need
"parted" to
> learn dealing with the bogus partition table.)
> 
>> My only other thought is that a simple set of tests could be good.
>> However it's not worth having tests that only test if __builtin*
>> functions are correct (hopefully GCC is already testing that).  So
>> tests would have to check the fallback macros are correct, even if
>> they are not used on the current platform.
> 
> So: the fallbacks need to be available (= built) in the source code
> unconditionally, so they can be directly called by the test suite. The
> actual "falling back" to them must be separate. Is that what you
mean?
... should this go into common/replacements, or common/utils? The former
seems like a better fit. On the other hand, libnbd (assuming we'll want
to port the same to libnbd) does not have common/replacements at all...

Thanks
Laszlo

On Tue, Nov 23, 2021 at 11:44:06AM +0100, Laszlo Ersek
wrote:
> On 11/22/21 23:31, Richard W.M. Jones wrote:
> > My only other thought is that a simple set of tests could be good.
> > However it's not worth having tests that only test if __builtin*
> > functions are correct (hopefully GCC is already testing that).  So
> > tests would have to check the fallback macros are correct, even if
> > they are not used on the current platform.
> 
> So: the fallbacks need to be available (= built) in the source code
> unconditionally, so they can be directly called by the test suite. The
> actual "falling back" to them must be separate. Is that what you
mean?
I just meant that if we have a test suite which tests the end result
(macros like "ADD_OVERFLOW"), because we will only do CI on modern
systems, it will only ever test that __builtin_add_overflow works.
That's pointless because presumably GCC already tests that, and the
fallback code would never be tested.  So the tests would need to
somehow check that the fallback path works.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/