From: Stefan Sperling <stsp@stsp.name>
Subject: Re: unsigned char cast for is* ctype.h macros
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 17 Nov 2022 10:09:26 +0100

On Thu, Nov 17, 2022 at 09:39:32AM +0100, Omar Polo wrote:
> this should 'fix' them all, except for the various parse.y.  A grep in
> usr.sbin shows that noone casts there.  (these calls are present in
> the parse.y files and not added by yacc.)
> 
> the ones in diff are maybe a bit redundant, except for one or two case
> all should already pass uint8_t and/or unsigned char, changing just in
> case in the future the types will change.
> 
> (if ok i'll commit the diff bits separately to diff.git and then sync
> as usual.)

Thanks for checking. I'm surprised we still have calls into
ctype functions which lack casts.

The diff.git file changes should be separated out and committed
to diff.git first.

ok stsp@

> diff /home/op/w/gotd
> commit - c2a4f618fa3c9683fbb5f384b117f7e45a041122
> path + /home/op/w/gotd
> blob - 238ecb9d0ee975efd8eacc55de14b8cfb69792de
> file + lib/diff_atomize_text.c
> --- lib/diff_atomize_text.c
> +++ lib/diff_atomize_text.c
> @@ -69,7 +69,7 @@ diff_data_atomize_text_lines_fd(struct diff_data *d)
>  			while (eol == 0 && i < r) {
>  				if (buf[i] != '\r' && buf[i] != '\n') {
>  					if (!ignore_whitespace
> -					    || !isspace(buf[i]))
> +					    || !isspace((unsigned char)buf[i]))
>  						hash = diff_atom_hash_update(
>  						    hash, buf[i]);
>  					if (buf[i] == '\0')
> @@ -142,7 +142,7 @@ diff_data_atomize_text_lines_mmap(struct diff_data *d)
>  
>  		while (line_end < end && *line_end != '\r' && *line_end != '\n') {
>  			if (!ignore_whitespace
> -			    || !isspace(*line_end))
> +			    || !isspace((unsigned char)*line_end))
>  				hash = diff_atom_hash_update(hash, *line_end);
>  			if (*line_end == '\0')
>  				embedded_nul = true;
> blob - 26fa788bccf6fc4e54a3c2efe5e6a389a1d34032
> file + lib/diff_main.c
> --- lib/diff_main.c
> +++ lib/diff_main.c
> @@ -62,11 +62,11 @@ buf_cmp(const unsigned char *left, size_t left_len,
>  			unsigned char cl = left[il];
>  			unsigned char cr = right[ir];
>  
> -			if (isspace(cl) && il < left_len) {
> +			if (isspace((unsigned char)cl) && il < left_len) {
>  				il++;
>  				continue;
>  			}
> -			if (isspace(cr) && ir < right_len) {
> +			if (isspace((unsigned char)cr) && ir < right_len) {
>  				ir++;
>  				continue;
>  			}
> @@ -80,12 +80,12 @@ buf_cmp(const unsigned char *left, size_t left_len,
>  		}
>  		while (il < left_len) {
>  			unsigned char cl = left[il++];
> -			if (!isspace(cl))
> +			if (!isspace((unsigned char)cl))
>  				return 1;
>  		}
>  		while (ir < right_len) {
>  			unsigned char cr = right[ir++];
> -			if (!isspace(cr))
> +			if (!isspace((unsigned char)cr))
>  				return -1;
>  		}
>  
> blob - a487dffb531fb051aa859f3c1ac13816fdfcd926
> file + lib/gitproto.c
> --- lib/gitproto.c
> +++ lib/gitproto.c
> @@ -54,13 +54,13 @@ tokenize_line(char **tokens, char *line, int len, int 
>  		tokens[i] = NULL;
>  
>  	for (i = 0; n < len && i < maxtokens; i++) {
> -		while (isspace(*line)) {
> +		while (isspace((unsigned char)*line)) {
>  			line++;
>  			n++;
>  		}
>  		p = line;
>  		while (*line != '\0' && n < len &&
> -		    (!isspace(*line) || i == maxtokens - 1)) {
> +		    (!isspace((unsigned char)*line) || i == maxtokens - 1)) {
>  			line++;
>  			n++;
>  		}
> blob - 114e0ca8c6f110af5f8ebfd87ac1cdd552fbc92c
> file + lib/object_parse.c
> --- lib/object_parse.c
> +++ lib/object_parse.c
> @@ -410,12 +410,14 @@ parse_gmtoff(time_t *gmtoff, const char *tzstr)
>  	else if (*p != '+')
>  		return got_error(GOT_ERR_BAD_OBJ_DATA);
>  	p++;
> -	if (!isdigit(*p) && !isdigit(*(p + 1)))
> +	if (!isdigit((unsigned char)*p) &&
> +	    !isdigit((unsigned char)*(p + 1)))
>  		return got_error(GOT_ERR_BAD_OBJ_DATA);
>  	h = (((*p - '0') * 10) + (*(p + 1) - '0'));
>  
>  	p += 2;
> -	if (!isdigit(*p) && !isdigit(*(p + 1)))
> +	if (!isdigit((unsigned char)*p) &&
> +	    !isdigit((unsigned char)*(p + 1)))
>  		return got_error(GOT_ERR_BAD_OBJ_DATA);
>  	m = ((*p - '0') * 10) + (*(p + 1) - '0');
>  
> blob - 52cedcef2588aa0c074d4d486c05e33a3bac7eb4
> file + lib/pkt.c
> --- lib/pkt.c
> +++ lib/pkt.c
> @@ -144,7 +144,7 @@ got_pkt_readpkt(int *outlen, int fd, char *buf, int bu
>  	if (chattygot > 1) {
>  		fprintf(stderr, "%s: readpkt: %zd:\t", getprogname(), n);
>  		for (i = 0; i < n; i++) {
> -			if (isprint(buf[i]))
> +			if (isprint((unsigned char)buf[i]))
>  				fputc(buf[i], stderr);
>  			else
>  				fprintf(stderr, "[0x%.2x]", buf[i]);
> @@ -179,7 +179,7 @@ got_pkt_writepkt(int fd, char *buf, int nbuf, int chat
>  	if (chattygot > 1) {
>  		fprintf(stderr, "%s: writepkt: %s:\t", getprogname(), len);
>  		for (i = 0; i < nbuf; i++) {
> -			if (isprint(buf[i]))
> +			if (isprint((unsigned char)buf[i]))
>  				fputc(buf[i], stderr);
>  			else
>  				fprintf(stderr, "[0x%.2x]", buf[i]);
> blob - 4fa55782d6cfa72a92ffedfb310e3ac0e508e55c
> file + libexec/got-fetch-pack/got-fetch-pack.c
> --- libexec/got-fetch-pack/got-fetch-pack.c
> +++ libexec/got-fetch-pack/got-fetch-pack.c
> @@ -208,7 +208,7 @@ fetch_error(const char *buf, size_t len)
>  	size_t i;
>  
>  	for (i = 0; i < len && i < sizeof(msg) - 1; i++) {
> -		if (!isprint(buf[i]))
> +		if (!isprint((unsigned char)buf[i]))
>  			return got_error_msg(GOT_ERR_BAD_PACKET,
>  			    "non-printable error message received from server");
>  		msg[i] = buf[i];
> blob - 7069d8b17d0928e260d85033e81ff7ac104c27dd
> file + libexec/got-send-pack/got-send-pack.c
> --- libexec/got-send-pack/got-send-pack.c
> +++ libexec/got-send-pack/got-send-pack.c
> @@ -181,7 +181,7 @@ send_error(const char *buf, size_t len)
>  	size_t i;
>  
>  	for (i = 0; i < len && i < sizeof(msg) - 1; i++) {
> -		if (!isprint(buf[i]))
> +		if (!isprint((unsigned char)buf[i]))
>  			return got_error_msg(GOT_ERR_BAD_PACKET,
>  			    "non-printable error message received from server");
>  		msg[i] = buf[i];
> 
>