"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

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

Download raw body.

Thread
  • Theo Buehler:

    [rfc] tog horizontal scroll (diff & blame view)

  • > @@ -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;
    > +	}
    
    
    
  • Theo Buehler:

    [rfc] tog horizontal scroll (diff & blame view)