compiler warning in pgcrypto imath.c

Started by Jeff Janesalmost 7 years ago7 messages
#1Jeff Janes
jeff.janes@gmail.com
1 attachment(s)

When compiling on an AWS 64 bit Arm machine, I get this compiler warning:

imath.c: In function 's_ksqr':
imath.c:2590:6: warning: variable 'carry' set but not used
[-Wunused-but-set-variable]
carry;
^~~~~

With this version():

PostgreSQL 12devel on aarch64-unknown-linux-gnu, compiled by gcc
(Ubuntu/Linaro 7.3.0-27ubuntu1~18.04) 7.3.0, 64-bit

The attached patch adds PG_USED_FOR_ASSERTS_ONLY to silence it. Perhaps
there is a better way, given that we want to change imath.c as little as
possible from its upstream?

Cheers,

Jeff

Attachments:

pgcrypto_warning.patchapplication/octet-stream; name=pgcrypto_warning.patchDownload
diff --git a/contrib/pgcrypto/imath.c b/contrib/pgcrypto/imath.c
index 92422aa3ad..bf399eebe5 100644
--- a/contrib/pgcrypto/imath.c
+++ b/contrib/pgcrypto/imath.c
@@ -18,6 +18,7 @@
  *    - rename DEBUG to IMATH_DEBUG
  *    - replace stdint.h usage with c.h equivalents
  *    - suppress MSVC warning 4146
+ *    - add required PG_USED_FOR_ASSERTS_ONLY
  *
  * 2. Download a newer imath.c and imath.h.  Transform them like in step 1.
  *    Apply to these files the diff you saved in step 1.  Look for new lines
@@ -2587,7 +2588,7 @@ s_ksqr(mp_digit *da, mp_digit *dc, mp_size size_a)
 		mp_digit   *t1,
 				   *t2,
 				   *t3,
-					carry;
+					carry PG_USED_FOR_ASSERTS_ONLY;
 		mp_size		at_size = size_a - bot_size;
 		mp_size		buf_size = 2 * bot_size;
 
#2Michael Paquier
michael@paquier.xyz
In reply to: Jeff Janes (#1)
Re: compiler warning in pgcrypto imath.c

On Fri, Mar 22, 2019 at 08:20:53PM -0400, Jeff Janes wrote:

PostgreSQL 12devel on aarch64-unknown-linux-gnu, compiled by gcc
(Ubuntu/Linaro 7.3.0-27ubuntu1~18.04) 7.3.0, 64-bit

Adding Noah in CC as he has done the update of imath lately.

The attached patch adds PG_USED_FOR_ASSERTS_ONLY to silence it. Perhaps
there is a better way, given that we want to change imath.c as little as
possible from its upstream?

Maybe others have better ideas, but marking the variable with
PG_USED_FOR_ASSERTS_ONLY as you propose seems like the least invasive
method of all.
--
Michael

#3Noah Misch
noah@leadboat.com
In reply to: Michael Paquier (#2)
Re: compiler warning in pgcrypto imath.c

On Sat, Mar 23, 2019 at 10:20:16AM +0900, Michael Paquier wrote:

On Fri, Mar 22, 2019 at 08:20:53PM -0400, Jeff Janes wrote:

PostgreSQL 12devel on aarch64-unknown-linux-gnu, compiled by gcc
(Ubuntu/Linaro 7.3.0-27ubuntu1~18.04) 7.3.0, 64-bit

Adding Noah in CC as he has done the update of imath lately.

The attached patch adds PG_USED_FOR_ASSERTS_ONLY to silence it. Perhaps
there is a better way, given that we want to change imath.c as little as
possible from its upstream?

Maybe others have better ideas, but marking the variable with
PG_USED_FOR_ASSERTS_ONLY as you propose seems like the least invasive
method of all.

That patch looks good. Thanks. The main alternative would be to pass
-Wno-unused for this file. Since you're proposing only one instance
PG_USED_FOR_ASSERTS_ONLY, I favor PG_USED_FOR_ASSERTS_ONLY over -Wno-unused.

#4Andres Freund
andres@anarazel.de
In reply to: Noah Misch (#3)
Re: compiler warning in pgcrypto imath.c

Hi Noah,

On 2019-03-23 00:02:36 -0700, Noah Misch wrote:

On Sat, Mar 23, 2019 at 10:20:16AM +0900, Michael Paquier wrote:

On Fri, Mar 22, 2019 at 08:20:53PM -0400, Jeff Janes wrote:

PostgreSQL 12devel on aarch64-unknown-linux-gnu, compiled by gcc
(Ubuntu/Linaro 7.3.0-27ubuntu1~18.04) 7.3.0, 64-bit

Adding Noah in CC as he has done the update of imath lately.

The attached patch adds PG_USED_FOR_ASSERTS_ONLY to silence it. Perhaps
there is a better way, given that we want to change imath.c as little as
possible from its upstream?

Maybe others have better ideas, but marking the variable with
PG_USED_FOR_ASSERTS_ONLY as you propose seems like the least invasive
method of all.

That patch looks good. Thanks. The main alternative would be to pass
-Wno-unused for this file. Since you're proposing only one instance
PG_USED_FOR_ASSERTS_ONLY, I favor PG_USED_FOR_ASSERTS_ONLY over -Wno-unused.

This is marked as an open item, owned by you. Could you commit the
patch or otherwise resovle the issue?

Greetings,

Andres Freund

#5Noah Misch
noah@leadboat.com
In reply to: Andres Freund (#4)
Re: compiler warning in pgcrypto imath.c

On Wed, May 01, 2019 at 09:18:02AM -0700, Andres Freund wrote:

On 2019-03-23 00:02:36 -0700, Noah Misch wrote:

On Sat, Mar 23, 2019 at 10:20:16AM +0900, Michael Paquier wrote:

On Fri, Mar 22, 2019 at 08:20:53PM -0400, Jeff Janes wrote:

PostgreSQL 12devel on aarch64-unknown-linux-gnu, compiled by gcc
(Ubuntu/Linaro 7.3.0-27ubuntu1~18.04) 7.3.0, 64-bit

Adding Noah in CC as he has done the update of imath lately.

The attached patch adds PG_USED_FOR_ASSERTS_ONLY to silence it. Perhaps
there is a better way, given that we want to change imath.c as little as
possible from its upstream?

Maybe others have better ideas, but marking the variable with
PG_USED_FOR_ASSERTS_ONLY as you propose seems like the least invasive
method of all.

That patch looks good. Thanks. The main alternative would be to pass
-Wno-unused for this file. Since you're proposing only one instance
PG_USED_FOR_ASSERTS_ONLY, I favor PG_USED_FOR_ASSERTS_ONLY over -Wno-unused.

This is marked as an open item, owned by you. Could you commit the
patch or otherwise resovle the issue?

I pushed Jeff's patch.

#6Michael Paquier
michael@paquier.xyz
In reply to: Noah Misch (#5)
Re: compiler warning in pgcrypto imath.c

On Sat, May 04, 2019 at 12:15:19AM -0700, Noah Misch wrote:

I pushed Jeff's patch.

Upon resolution, could you move the related open item on the wiki
page to the list of resolved issues [1]https://wiki.postgresql.org/wiki/PostgreSQL_12_Open_Items#resolved_before_12beta1 -- Michael?

[1]: https://wiki.postgresql.org/wiki/PostgreSQL_12_Open_Items#resolved_before_12beta1 -- Michael
--
Michael

#7Jeff Janes
jeff.janes@gmail.com
In reply to: Noah Misch (#5)
Re: compiler warning in pgcrypto imath.c

On Sat, May 4, 2019 at 3:15 AM Noah Misch <noah@leadboat.com> wrote:

I pushed Jeff's patch.

Thank you. I've re-tested it and I get warning-free compilation now.

Cheers,

Jeff