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