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

[Eric Blake]

On Fri, Nov 19, 2021 at 03:15:58PM +0000, Richard W.M. Jones wrote:
> On Fri, Nov 19, 2021 at 03:35:48PM +0100, Laszlo Ersek wrote:
> > (1) What is the usual style in nbdkit to highlight parameters of
> > functions and function-like macros in documentation (comments)? Here I
> > used parentheses. Normally I use quotes "".
> 
> We don't have one, so quotes are fine.
> 
> > (2) In the actual functions, we return "true" for "overflow". My
> > original conditions expressed "no overflow". In this source code, I
> > *open-coded* the negations of my original conditions, using De Morgan's
> > laws. That's because I dislike expressions with an outermost "!"
> > operator -- whenever I face such an expression, I always work the
> > outermost negation *into* the formula, as deeply as I can.
> > 
> > However, I realize the resultant conditions may not be easy to read this
> > way. Here's an alternative for the body of check_add_overflow(), based
> > on the original formulation:
> > 
> >   register bool in_range;

I generally avoid 'register' in my C code; the compiler is smart
enough to figure that out without me having to do extra typing.

> > 
> >   *r = a + b;
> >   in_range = a <= UINTMAX_MAX - b && *r <= max;
> >   return !in_range;
> 
> Seems reasonable.

I can go with either form.

> 
> > (3) In the overflow conditions, I used parentheses as sparingly as
> > possible; only when needed. I could also use as many as possible.
> > 
> > - Compare, for the addition:
> > 
> >   return ((a > UINTMAX_MAX - b) || (*r > max));
> > 
> > or
> > 
> >   in_range = ((a <= UINTMAX_MAX - b) && (*r <= max));
> >   return !in_range;
> > 
> > - Compare, for the multiplication:
> > 
> >   return ((b > 0) && ((a > UINTMAX_MAX / b) || (*r > max)));
> > 
> > or
> > 
> >   in_range = ((b == 0) || ((a <= UINTMAX_MAX / b) && (*r <= max)));
> >   return !in_range;
> > 
> > What's the preferred form?
> 
> Personally I use fewest parens, but indicate precedence by dropping
> whitespace, but I guess whatever works.  However you definitely don't
> need the parens around the entire return value, C doesn't require
> that.

I use extra () when using &, |, << and >> (as those are less frequent
operators), but for +, *, <, >, &&, ||, and ==, I generally assume the
precedence is well-known enough to elide spurious ().

I've seen other projects that use code linters to enforce a particular
style.  We don't do that here, which means we can probably find
exceptions to a number of style suggestions, but it hasn't been enough
to bother me.

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