Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I have pushed this now, hoping it won't explode.
mamba is not happy:
ccache cc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute-Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security
-fno-strict-aliasing-fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -Werror
-fPIC-DPIC -fvisibility=hidden -I. -I. -I../../src/include -I/usr/pkg/include/libxml2 -I/usr/pkg/include
-I/usr/pkg/include -I/usr/pkg/include -c -o pgcrypto.o pgcrypto.c
In file included from /usr/include/ctype.h:100,
from ../../src/include/port.h:16,
from ../../src/include/c.h:1345,
from ../../src/include/postgres.h:48,
from crypt-sha.c:46:
crypt-sha.c: In function 'px_crypt_shacrypt':
crypt-sha.c:324:16: error: array subscript has type 'char' [-Werror=char-subscripts]
324 | if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
| ^
crypt-sha.c:324:32: error: array subscript has type 'char' [-Werror=char-subscripts]
324 | if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
| ^
cc1: all warnings being treated as errors
gmake[1]: *** [<builtin>: crypt-sha.o] Error 1
What this is on about is that portable use of isalpha() or isdigit()
requires casting a "char" value to "unsigned char". I was about to
make that simple change when I started to question if we actually
want to be using <ctype.h> here at all. Do you REALLY intend that
any random non-ASCII letter is okay to include in the decoded_salt?
Are you okay with that filter being locale-dependent?
I'd be more comfortable with a check like
if (strchr("...valid chars...", *ep) != NULL)
It looks like "_crypt_itoa64" might be directly usable as the
valid-chars string, too. (BTW, why is _crypt_itoa64 not
marked const?)
regards, tom lane
