From: Theo Buehler <tb@theobuehler.org>
Subject: Re: [rfc] tog horizontal scroll (diff & blame view)
To: Mark Jamsek <mark@jamsek.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 16 Jun 2022 10:25:11 +0200

> @@ -1223,21 +1224,62 @@ done:
>  	return err;
>  }
> 
> +static const struct got_error *
> +expand_tab(char **ptr, const char *src)
> +{
> +	char	*dst;
> +	size_t	 len, n, idx = 0, sz = 0;
> +
> +	*ptr = NULL;
> +	n = len = strlen(src);
> +	dst = malloc((n + 1) * sizeof(char));

This looks like a potential overflow due to multiplying sizes in malloc().
Since sizeof(char) == 1 by definition that's not an issue. For the same
reason, I would simplify this to

	dst = malloc(n + 1);

> +	if (dst == NULL)
> +		return got_error_from_errno("malloc");
> +
> +	while (idx < len && src[idx]) {
> +		const char c = src[idx];
> +
> +		if (c == '\t') {
> +			size_t nb = TABSIZE - sz % TABSIZE;
> +			n += nb;
> +			dst = reallocarray(dst, n, sizeof(char));
> +			if (dst == NULL)
> +				return got_error_from_errno("reallocarray");

This leaks dst on reallocarray() failure and should use an intermediate
pointer. Also, I think it would be better to grow n only after the
reallocation succeeded:

			p = realloc(dst, n + nb);
			if (p == NULL) {
				free(dst);
				return got_error_from_errno("realloc");
			}
			dst = p;
			n += nb;


> +			memcpy(dst + sz, "        ", nb);

I would use
			memset(dst + sz, ' ', nb);

> +			sz += nb;
> +		} else
> +			dst[sz++] = src[idx];
> +		++idx;
> +	}