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

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/

On Tue, Nov 23, 2021 at 03:25:22PM +0000, Richard W.M. Jones
wrote:
> 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.
Something along the lines of:

#if (...gcc or clang new enough...) && !defined(NBDKIT_FALLBACK)
# define ADD_OVERFLOW(a, b, r) __builtin_add_overflow((a), (b), (r))
#else
# define ADD_OVERFLOW(a, b, r) ...fallback...
#endif

so that the testsuite can then #define NBDKIT_FALLBACK for testing the
fallback code even on new enough compilers.  I'm not sure how best to
namespace the witness macro for forcing a fallback if the code is
copied between nbdkit and libnbd.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org