Обсуждение: fix warnings in 9.6, 10, 11's contrib when compiling without openssl
Hello,
after plain ./configure without options (including noticeable absence of
--with-openssl) and make,
make -C contrib COPT=-Werror gives error with gcc-11 on REL_9_6_STABLE,
REL_10_STABLE or REL_11_STABLE.
The warning/error is about misleading indent in
contrib/pgcrypto/imath.c's CLAMP macro:
imath.c: In function ‘mp_int_add’:
imath.c:133:1: error: this ‘while’ clause does not guard...
[-Werror=misleading-indentation]
133 | while(uz_ > 1 && (*dz_-- == 0)) --uz_;MP_USED(z_)=uz_;}while(0)
| ^~~~~
imath.c:670:17: note: in expansion of macro ‘CLAMP’
670 | CLAMP(c);
| ^~~~~
In file included from imath.c:34:
imath.h:68:26: note: ...this statement, but the latter is misleadingly
indented as if it were guarded by the ‘while’
68 | #define MP_USED(Z) ((Z)->used)
| ^
imath.c:133:39: note: in expansion of macro ‘MP_USED’
133 | while(uz_ > 1 && (*dz_-- == 0)) --uz_;MP_USED(z_)=uz_;}while(0)
| ^~~~~~~
imath.c:670:17: note: in expansion of macro ‘CLAMP’
670 | CLAMP(c);
| ^~~~~
pgcrypto-fix-imath-clamp-warning.patch, attached, is a suggested minimal
fix:
diff --git a/contrib/pgcrypto/imath.c b/contrib/pgcrypto/imath.c
index b94a51b81a4..801b843cbe3 100644
--- a/contrib/pgcrypto/imath.c
+++ b/contrib/pgcrypto/imath.c
@@ -130,7 +130,8 @@ do{T *u_=(A),*v_=u_+(N)-1;while(u_<v_){T
xch=*u_;*u_++=*v_;*v_--=xch;}}while(0)
#else
#define CLAMP(Z) \
do{mp_int z_=(Z);mp_size uz_=MP_USED(z_);mp_digit
*dz_=MP_DIGITS(z_)+uz_-1;\
-while(uz_ > 1 && (*dz_-- == 0)) --uz_;MP_USED(z_)=uz_;}while(0)
+while(uz_ > 1 && (*dz_-- == 0)) --uz_;\
+MP_USED(z_)=uz_;}while(0)
#endif
#undef MIN
The same patch works for all 9.6, 10 and 11. It's not needed in 12 or
later, they compile without warnings just fine even without --with-openssl.
In addition, 9.6 (unlike 10 and 11) gives four "array-parameter="
warnings about contrib/pgcrypto/sha2.c:
sha2.c:552:20: error: argument 1 of type ‘uint8[]’ {aka ‘unsigned
char[]’} with mismatched bound [-Werror=array-parameter=]
552 | SHA256_Final(uint8 digest[], SHA256_CTX *context)
| ~~~~~~^~~~~~~~
In file included from sha2.c:44:
sha2.h:93:30: note: previously declared as ‘uint8[32]’ {aka ‘unsigned
char[32]’}
93 | void SHA256_Final(uint8[SHA256_DIGEST_LENGTH],
SHA256_CTX *);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
and the same for SHA{512,384,224}_Final.
pgcrypto-fix-sha2-warning.patch, attached, is a suggested minimal fix
for that:
void
-SHA256_Final(uint8 digest[], SHA256_CTX *context)
+SHA256_Final(uint8 digest[SHA256_DIGEST_LENGTH], SHA256_CTX *context)
{
etc.
Please consider fixing those warnings.
--
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru
Вложения
> On 10 Nov 2021, at 11:14, Anton Voloshin <a.voloshin@postgrespro.ru> wrote: > The warning/error is about misleading indent in contrib/pgcrypto/imath.c's CLAMP macro: This is upstream code in a stable branch formatted by pgindent at the time of the release, I'm not sure the churn is worth it for fixing warnings for non-standard compiler options here. I don't see any of these warnings skimming logs in the buildfarm. > In addition, 9.6 (unlike 10 and 11) gives four "array-parameter=" warnings about contrib/pgcrypto/sha2.c: 9.6 will be EOL from the release tomorrow, no more fixes are applied to that branch. -- Daniel Gustafsson https://vmware.com/
On Wed, Nov 10, 2021 at 02:56:00PM +0100, Daniel Gustafsson wrote: > This is upstream code in a stable branch formatted by pgindent at the time of > the release, I'm not sure the churn is worth it for fixing warnings for > non-standard compiler options here. I don't see any of these warnings skimming > logs in the buildfarm. I would suggest to send any improvements for imath directly to upstream, then Postgres would just feed on that when we update it: https://github.com/creachadair/imath Peter has removed this code as of db7d1a7, but there could still be a point in updating imath on the stable branches where it exists if there is an issue with it impacting pgcrypto without OpenSSL. -- Michael
Вложения
On 11/11/2021 08:14, Michael Paquier wrote: > I would suggest to send any improvements for imath directly to > upstream, then Postgres would just feed on that when we update it: > https://github.com/creachadair/imath I didn't realise imath comes (occasionally) from external repo. Thank you. The compilation warnings are only relevant for their old releases, before imath v1.27 where they've replaced macros with inline functions, see https://github.com/creachadair/imath/commit/d4760eef8 So imath in upstream doesn't require updates (especially since those warnings come specifically from pgindent's work, not from the upstream source). But Postgres' 10_STABLE and 11_STABLE still use old version with CLAMP as a macro. Noah Misch was last to update imath from upstream in February 2019 and that change became part of REL_12_STABLE and later: > Import changes from IMath versions (1.3, 1.29]. https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=48e24ba6b7fd3bfd156b51e8d768fd48df0d288b Noah and others, do you think it would be a good idea to update imath in REL_10_STABLE and REL_11_STABLE to a later upstream version to avoid those warnings about misleading indent? (see beginning of this thread) Admittedly, those warnings can only be seen in unusual circumstances (no openssl), but still, as you have mentioned in your commit message, > We're better off naively tracking upstream than reactively > maintaining a twelve-year-old snapshot of upstream. Perhaps even update imath in all relevant stable branches, 10..14 inclusive, to the same upstream version? On 11/11/2021 08:14, Michael Paquier wrote: > Peter has removed this code as of db7d1a7, but there could still be a > point in updating imath on the stable branches where it exists if > there is an issue with it impacting pgcrypto without OpenSSL. Perhaps that would be a good idea, see above. -- Anton Voloshin https://postgrespro.ru Postgres Professional, The Russian Postgres Company