Re: clang warning about mini-gmp.c when used in Emacs

[Paul Eggert]

On 4/14/22 22:29, Niels Möller wrote:


1. Is the void cast really needed?


Yes, I think so. Various compilers complain if a statement 'E;' has an 
expression E without side effects. The standard way to pacify them is to 
cast E to void. See, for example, 
.


mini-gmp.c is different from gmp-impl.h, because it's intended to be 
used by lots of downstream projects which may use compilers that 
gmp-impl.h does not use. This may help to explain why it needs casts 
that gmp-impl.h doesn't need.




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)


Yes, thanks, that should work too.
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

clang warning about mini-gmp.c when used in Emacs

[Paul Eggert]

Mattias Engdegård reported that the Emacs master source currently 
generates the following warning when Emacs is built with mini-gmp.c 
under Clang 13:



In file included from /Users/mattias/emacs/lib/mini-gmp-gnulib.c:47:
/Users/mattias/emacs/lib/mini-gmp.c:1138:2: warning: unused variable '__cy' 
[-Wunused-variable]
gmp_assert_nocarry (mpn_rshift (np, np, dn, shift));
^
/Users/mattias/emacs/lib/mini-gmp.c:91:15: note: expanded from macro 
'gmp_assert_nocarry'
mp_limb_t __cy = (x);  \


The problem occurs because assert(X) does not evaluate X when NDEBUG is 
defined. Proposed patch attached.


Another advantage of this patch is that any messages generated by 
assertion failures are more informative.diff -r d45103d658ca mini-gmp/mini-gmp.c
--- a/mini-gmp/mini-gmp.c	Wed Mar 30 23:16:18 2022 +0200
+++ b/mini-gmp/mini-gmp.c	Fri Apr 08 16:18:06 2022 -0700
@@ -87,10 +87,11 @@
 #define GMP_MPN_OVERLAP_P(xp, xsize, yp, ysize)\
   ((xp) + (xsize) > (yp) && (yp) + (ysize) > (xp))
 
-#define gmp_assert_nocarry(x) do { \
-mp_limb_t __cy = (x);	   \
-assert (__cy == 0);		   \
-  } while (0)
+#ifdef NDEBUG
+# define gmp_assert_nocarry(x) ((void) (x))
+#else
+# define gmp_assert_nocarry(x) assert ((x) == 0)
+#endif
 
 #define gmp_clz(count, x) do {		\
 mp_limb_t __clz_x = (x);		\
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

gmp-bugs mail archive status?

[Paul Eggert]

While following up on my recent mini-gmp.c bug report I noticed that 
https://gmplib.org/list-archives/gmp-bugs/ stops in November 2021 so I 
couldn't give a URL to a recent message.


Perhaps you've moved to a different mail archiver? or you've disabled 
archiving? Or you update the archive only occasionally? As a suggestion, 
it'd help to modify the main GMP page  
to indicate what's going on, as I was confused at first.


Thanks.
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

Re: unused macro MPN_PTR_SWAP in mini-gmp.c

[Paul Eggert]

On 3/19/22 04:44, Niels Möller wrote:

Paul Eggert  writes:


In libgmp 6.2.1, mini-gmp.c defines a macro without using it, causing
the command "gcc -Wunused-macros -Werror" to fail with the following
diagnostic:


I haven't used -Wunused-macros before, I take it it warns only about
macros defined in .c files? Since it's perfectly normal that macros
defined in .h files are unused in some of the compilation units
including the .h file?


Yes, that's right.



mini-gmp.c is a bit special, in that one more-or-less recommended
usecase is to #include this file into some other compilation unit. E.g.,
like in https://gmplib.org/repo/gmp/file/tip/bootstrap.c#l33.

But probably still shouln't define macros not used within mini-gmp.c;
any additional utility macros should probably go in mini-gmp.h, and be
compatible with public macros from gmp.h.


MPN_PTR_SWAP is an internal macro, though, and I expect you don't want 
it to be public.


A couple of days ago Marco gave a simple fix that looks like a win, as 
it also makes mini-gmp.c shorter and easier to read; please see 
attachment. Could you please install that? Thanks.--- Begin Message ---

Ciao Paul,

Il 2022-03-09 02:56 Paul Eggert ha scritto:

In libgmp 6.2.1, mini-gmp.c defines a macro without using it, causing


Thanks for spotting this!


Proposed patch attached.


There is also a different possible way to heal the problem: using it :-D

diff -r a44f487c8d20 mini-gmp/mini-gmp.c
--- a/mini-gmp/mini-gmp.c   Fri Mar 11 21:13:20 2022 +0100
+++ b/mini-gmp/mini-gmp.c   Thu Mar 17 19:15:51 2022 +0100
@@ -1938,8 +1938,7 @@
 mpz_swap (mpz_t u, mpz_t v)
 {
   MP_SIZE_T_SWAP (u->_mp_size, v->_mp_size);
-  MP_SIZE_T_SWAP (u->_mp_alloc, v->_mp_alloc);
-  MP_PTR_SWAP (u->_mp_d, v->_mp_d);
+  MPN_PTR_SWAP (u->_mp_d, u->_mp_alloc, v->_mp_d, v->_mp_alloc);
 }



Ĝis,
m
--- End Message ---
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

Re: unused macro MPN_PTR_SWAP in mini-gmp.c

[Paul Eggert]

On 3/17/22 11:32, Marco Bodrato wrote:

  mpz_swap (mpz_t u, mpz_t v)
  {
    MP_SIZE_T_SWAP (u->_mp_size, v->_mp_size);
-  MP_SIZE_T_SWAP (u->_mp_alloc, v->_mp_alloc);
-  MP_PTR_SWAP (u->_mp_d, v->_mp_d);
+  MPN_PTR_SWAP (u->_mp_d, u->_mp_alloc, v->_mp_d, v->_mp_alloc);


Thank you, this looks good.
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

unused macro MPN_PTR_SWAP in mini-gmp.c

[Paul Eggert]

In libgmp 6.2.1, mini-gmp.c defines a macro without using it, causing 
the command "gcc -Wunused-macros -Werror" to fail with the following 
diagnostic:


lib/mini-gmp.c:251: warning: macro "MPN_PTR_SWAP" is not used 
[-Wunused-macros]
  251 | #define MPN_PTR_SWAP(xp,xs, yp,ys) 
\

  |

Proposed patch attached.--- mini-gmp/mini-gmp.c-bak	2020-11-14 10:45:09.0 -0800
+++ mini-gmp/mini-gmp.c	2022-03-08 17:50:48.637569187 -0800
@@ -247,11 +247,6 @@ see https://www.gnu.org/licenses/.  */
 (y) = __mp_srcptr_swap__tmp;	\
   } while (0)
 
-#define MPN_PTR_SWAP(xp,xs, yp,ys)	\
-  do {	\
-MP_PTR_SWAP (xp, yp);		\
-MP_SIZE_T_SWAP (xs, ys);		\
-  } while(0)
 #define MPN_SRCPTR_SWAP(xp,xs, yp,ys)	\
   do {	\
 MP_SRCPTR_SWAP (xp, yp);		\
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

GMP bootstrap fails with Autoconf 2.69d

[Paul Eggert]

Autoconf 2.70 is about to be released, and some people have been using 
prereleases to build lots of GNU packages. They've run into a problem with GMP, 
though, because GMP uses AC_INIT in a way contrary to the Autoconf manual, and 
changes in Autoconf mean that GMP's "./.bootstrap" procedure fails as follows:


$ ./.bootstrap
/usr/bin/m4:configure.ac:40: Warning: excess arguments to builtin `m4_define' 
ignored

autom4te: error: /usr/bin/m4 failed with exit status: 1
aclocal: error: echo failed with exit status: 1
autoreconf: error: aclocal failed with exit status: 1

The bug occurs in the following line of configure.ac:

AC_INIT(GNU MP, GMP_VERSION, [gmp-bugs@gmplib.org, see 
https://gmplib.org/manual/Reporting-Bugs.html], gmp)


The Autoconf manual has long said that the 3rd argument of AC_INIT must be an 
email address. Unfortunately commas are not valid in RFC 5322 email addresses, 
and in Autoconf 2.69d the comma causes problems because a submacro interprets it 
as an argument separator. If you'd like to reproduce the bug, you can get 
Autoconf 2.69d as described here:


https://lists.gnu.org/archive/html/autoconf/2020-11/msg3.html

To fix the problem, I suggest using just [gmp-bugs@gmplib.org] as the 3rd 
argument of AC_INIT, as being the simplest; or if you want the commentary, put 
it in as valid RFC 5322 commentary, e.g., by parenthesizing it:


diff -r 18875a0a3f89 configure.ac
--- a/configure.ac  Tue Nov 10 18:17:13 2020 +0100
+++ b/configure.ac  Wed Nov 11 22:25:59 2020 -0800
@@ -37,7 +37,7 @@

 AC_REVISION($Revision$)
 AC_PREREQ(2.59)
-AC_INIT(GNU MP, GMP_VERSION, [gmp-bugs@gmplib.org, see 
https://gmplib.org/manual/Reporting-Bugs.html], gmp)
+AC_INIT(GNU MP, GMP_VERSION, [gmp-bugs@gmplib.org (see 
https://gmplib.org/manual/Reporting-Bugs.html)], gmp)

 AC_CONFIG_SRCDIR(gmp-impl.h)
 m4_pattern_forbid([^[ \t]*GMP_])
 m4_pattern_allow(GMP_LDFLAGS)


This should be portable to both older and newer Autoconf releases.

If you use Autoconf 2.69d you'll see that it also warns about a lot of obsolete 
stuff in gmp's configure.ac etc, but none of these warnings stop the build and 
they can be dealt with later.

___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

mini-gmp mpz_out_str dereferences null pointer with out-of-range base

[Paul Eggert]

The mpz_out_str implementation in mini-gmp.c computes strlen (NULL) if its base 
argument is out of range. This causes GCC 10.1 -fanalyzer to complain:


mini-gmp.c:4428:9: error: use of NULL 'str' where non-null expected [CWE-690] 
[-Wanalyzer-null-argument]


Proposed patch attached. This patch also fixes an unrelated double-negative in a 
comment that confused me on first reading.
diff -r c5d0fcb06969 mini-gmp/mini-gmp.c
--- a/mini-gmp/mini-gmp.c	Sat Jul 04 23:15:41 2020 +0200
+++ b/mini-gmp/mini-gmp.c	Thu Jul 09 12:27:11 2020 -0700
@@ -32,7 +32,7 @@
 
 /* NOTE: All functions in this file which are not declared in
mini-gmp.h are internal, and are not intended to be compatible
-   neither with GMP nor with future versions of mini-gmp. */
+   with GMP or with future versions of mini-gmp. */
 
 /* Much of the material copied from GMP files, including: gmp-impl.h,
longlong.h, mpn/generic/add_n.c, mpn/generic/addmul_1.c,
@@ -4425,6 +4425,8 @@
   size_t len, n;
 
   str = mpz_get_str (NULL, base, x);
+  if (!str)
+return 0;
   len = strlen (str);
   n = fwrite (str, 1, len, stream);
   gmp_free (str, len + 1);
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs

whitespace and grammar fixes for mini-gmp.c

[Paul Eggert]

When merging GNU MP 6.2.0 mini-gmp.c into GNU Emacs (which uses mini-gmp 
on platforms lacking libgmp), I noticed three trivial fixes that had 
been applied on the Emacs copy that would be useful to propagate 
upstream to GNU MP. The first is trailing whitespace on a source code 
line (which the Emacs version-control system warns about). The other two 
are duplicated words "the the" and "to to", respectively. Please see the 
attached patch.
diff -r a615debc4e93 mini-gmp/mini-gmp.c
--- a/mini-gmp/mini-gmp.c	Fri Jan 24 04:38:57 2020 +0100
+++ b/mini-gmp/mini-gmp.c	Mon Jan 27 16:08:00 2020 -0800
@@ -790,7 +790,7 @@
 
qh' = floor( (b^3 - 1) / u) - b = floor ((b^3 - b u - 1) / u
 	   = floor( (b (~u) + b-1) / u),
-	   
+
and the remainder
 
r = b (~u) + b-1 - qh (b uh + ul)
@@ -3418,7 +3418,7 @@
   gmp_lucas_step_k_2k (V, Qk, n);
 
   /* A step k->k+1 is performed if the bit in $n$ is 1	*/
-  /* mpz_tstbit(n,bs) or the the bit is 0 in $n$ but	*/
+  /* mpz_tstbit(n,bs) or the bit is 0 in $n$ but	*/
   /* should be 1 in $n+1$ (bs == b0)			*/
   if (b0 == bs || mpz_tstbit (n, bs))
 	{
@@ -4488,7 +4488,7 @@
   ptrdiff_t word_step;
   /* The current (partial) limb. */
   mp_limb_t limb;
-  /* The number of bytes left to to in this limb. */
+  /* The number of bytes left to do in this limb. */
   size_t bytes;
   /* The index where the limb was read. */
   mp_size_t i;
___
gmp-bugs mailing list
gmp-bugs@gmplib.org
https://gmplib.org/mailman/listinfo/gmp-bugs