*ping* Any feedback for the below patch is appreciated.
Thanks, On Sat, Feb 01, 2020 at 05:01:48PM +0100, Hiltjo Posthuma wrote: > Hi, > > This (pedantic) patch fixes a few casts for ctype functions/macros, like > isspace, isalpha and isdigit. > > The isspace(3) man page says: > > "CAVEATS > The argument c must be EOF or representable as an unsigned char; > otherwise, the result is undefined." > > > POSIX says: > > "The c argument is an int, the value of which the application shall ensure is > a > character representable as an unsigned char or equal to the value of the macro > EOF. If the argument has any other value, the behavior is undefined." > > Reference: > https://pubs.opengroup.org/onlinepubs/9699919799/functions/isspace.html > > > In practise this might affect the portable version for NetBSD. I think most > other implementations (OpenBSD, FreeBSD, Linux glibc, musl) (according to my > testing) do an implicit cast already. > > > To do a quick grep for similar cases: > grep -E > '(isalnum|isascii|isblank|iscntrl|isdigit|isgraph|islower|isprint|ispunct|isspace|isupper|isxdigit|tolower|toupper)' > *.{c,h} | grep '(int' > > > Patch below: > > > diff --git usr.sbin/smtpd/mta_session.c usr.sbin/smtpd/mta_session.c > index 9459d69275f..afd379b794c 100644 > --- usr.sbin/smtpd/mta_session.c > +++ usr.sbin/smtpd/mta_session.c > @@ -1296,9 +1296,9 @@ mta_io(struct io *io, int evt, void *arg) > (void)strlcat(s->replybuf, line, sizeof > s->replybuf); > else { > line = line + 4; > - if (isdigit((int)*line) && *(line + 1) == '.' && > - isdigit((int)*line+2) && *(line + 3) == '.' > && > - isdigit((int)*line+4) && > isspace((int)*(line + 5))) > + if (isdigit((unsigned char)*line) && *(line + > 1) == '.' && > + isdigit((unsigned char)*line+2) && *(line + > 3) == '.' && > + isdigit((unsigned char)*line+4) && > isspace((unsigned char)*(line + 5))) > (void)strlcat(s->replybuf, line+5, > sizeof s->replybuf); > else > (void)strlcat(s->replybuf, line, sizeof > s->replybuf); > @@ -1311,9 +1311,9 @@ mta_io(struct io *io, int evt, void *arg) > */ > if (s->replybuf[0] != '\0') { > p = line + 4; > - if (isdigit((int)*p) && *(p + 1) == '.' && > - isdigit((int)*p+2) && *(p + 3) == '.' && > - isdigit((int)*p+4) && isspace((int)*(p + 5))) > + if (isdigit((unsigned char)*p) && *(p + 1) == '.' && > + isdigit((unsigned char)*p+2) && *(p + 3) == '.' && > + isdigit((unsigned char)*p+4) && isspace((unsigned > char)*(p + 5))) > p += 5; > if (strlcat(s->replybuf, p, sizeof s->replybuf) >= > sizeof s->replybuf) > (void)strlcpy(s->replybuf, line, sizeof > s->replybuf); > diff --git usr.sbin/smtpd/parse.y usr.sbin/smtpd/parse.y > index eaa465ae83a..5cfd41092ee 100644 > --- usr.sbin/smtpd/parse.y > +++ usr.sbin/smtpd/parse.y > @@ -529,7 +529,7 @@ SMTP LIMIT limits_smtp > free($3); > YYERROR; > } > - if (isspace((int)*$3) || !isprint((int)*$3) || *$3== '@') { > + if (isspace((unsigned char)*$3) || !isprint((unsigned char)*$3) || > *$3== '@') { > yyerror("sub-addr-delim uses invalid character"); > free($3); > YYERROR; > diff --git usr.sbin/smtpd/smtp_client.c usr.sbin/smtpd/smtp_client.c > index 22e798900cf..48ec3e1d8c7 100644 > --- usr.sbin/smtpd/smtp_client.c > +++ usr.sbin/smtpd/smtp_client.c > @@ -779,9 +779,9 @@ smtp_client_replycat(struct smtp_client *proto, const > char *line) > line += 3; > if (line[0]) { > line += 1; > - if (isdigit((int)line[0]) && line[1] == '.' && > - isdigit((int)line[2]) && line[3] == '.' && > - isdigit((int)line[4]) && isspace((int)line[5])) > + if (isdigit((unsigned char)line[0]) && line[1] == '.' && > + isdigit((unsigned char)line[2]) && line[3] == '.' && > + isdigit((unsigned char)line[4]) && > isspace((unsigned char)line[5])) > line += 5; > } > } else > diff --git usr.sbin/smtpd/util.c usr.sbin/smtpd/util.c > index f59ad1e4690..77efe690336 100644 > --- usr.sbin/smtpd/util.c > +++ usr.sbin/smtpd/util.c > @@ -489,7 +489,7 @@ valid_domainpart(const char *s) > return res_hnok(s); > } > > -#define LABELCHR(c) ((c) == '-' || (c) == '_' || isalpha((int)(c)) || > isdigit((int)(c))) > +#define LABELCHR(c) ((c) == '-' || (c) == '_' || isalpha((unsigned char)(c)) > || isdigit((unsigned char)(c))) > #define LABELMAX 63 > #define DNAMEMAX 253 > > > -- > Kind regards, > Hiltjo > -- Kind regards, Hiltjo
