Re: [PATCH 06/10] diff.c: convert trivial snprintf calls to xsnprintf

From: Jeff King <peff@peff.net>
Date: Jun 3, 2016, 09:03 UTC
Message-ID: <20160603090320.GE28401@sigill.intra.peff.net>
In-Reply-To: <20160603074724.12173-6-gitter.spiros@gmail.com>

On Fri, Jun 03, 2016 at 07:47:20AM +0000, Elia Pinto wrote:

Show 16 quoted lines

>  diff.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/diff.c b/diff.c
> index d3734d3..fb61539 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4491,7 +4491,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
>  		len1 = remove_space(p->one->path, strlen(p->one->path));
>  		len2 = remove_space(p->two->path, strlen(p->two->path));
>  		if (p->one->mode == 0)
> -			len1 = snprintf(buffer, sizeof(buffer),
> +			len1 = xsnprintf(buffer, sizeof(buffer),
>  					"diff--gita/%.*sb/%.*s"
>  					"newfilemode%06o"
>  					"---/dev/null"

More that assume that PATH_MAX is plenty big for any patch we would see. I'd argue these should be on the heap, which removes that restriction, and gets rid of the "PATH_MAX * 4 + 20" magic at the top of the function.

It looks like the buffer just gets fed into sha1, so I was also tempted to suggest we simply feed the bits of string in directly. But we are using the format string here to handle the mode, so you'd still have to deal with that.

-Peff