On 2022-04-15 07:29:34 +0200, Niels Möller wrote: > I have two minor comments on this proposed fix: > > 1. Is the void cast really needed? Corresponding macros in gmp-impl.h > are defined like > > #if WANT_ASSERT > #define ASSERT_CARRY(expr) ASSERT_ALWAYS ((expr) != 0) > #define ASSERT_NOCARRY(expr) ASSERT_ALWAYS ((expr) == 0) > #else > #define ASSERT_CARRY(expr) (expr) > #define ASSERT_NOCARRY(expr) (expr) > #endif
I'd say that a cast to void is better in order to ensure that ASSERT_CARRY(expr) and ASSERT_NOCARRY(expr) have type void whether WANT_ASSERT is defined or not, so that type issues could be detected more easily. > 2. I'm a bit uneasy about side effects in the argument to the standard > assert macro, even with the NDEBUG ifdefs. Ditto. It seems that the ISO C standard doesn't say anything if NDEBUG not defined (the expression might be evaluated several times, like with any macro). > Alternative patch: > > --- a/mini-gmp/mini-gmp.c Tue Feb 15 09:18:40 2022 +0100 > +++ b/mini-gmp/mini-gmp.c Fri Apr 15 07:20:40 2022 +0200 > @@ -90,6 +90,7 @@ see https://www.gnu.org/licenses/. */ > #define gmp_assert_nocarry(x) do { \ > mp_limb_t __cy = (x); \ > assert (__cy == 0); \ > + (void) __cy; \ > } while (0) I prefer this patch. -- Vincent Lefèvre <vinc...@vinc17.net> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon) _______________________________________________ gmp-bugs mailing list gmp-bugs@gmplib.org https://gmplib.org/mailman/listinfo/gmp-bugs