Subject: Re: [PATCH 22/67] entry.c: convert strcpy to xsnprintf

From: Ramsay Jones <[email-masked]>
Date: Tue, 15 Sep 2015 20:01:01 +0100

On 15/09/15 16:40, Jeff King wrote:
> This particular conversion is non-obvious, because nobody
> has passed our function the length of the destination
> buffer. However, the interface to checkout_entry specifies
> that the buffer must be at least TEMPORARY_FILENAME_LENGTH
> bytes long, so we can check that (meaning the existing code
> was not buggy, but merely worrisome to somebody reading it).
>
> Signed-off-by: Jeff King <[email-masked]>
> ---
>  entry.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/entry.c b/entry.c
> index 1eda8e9..582c400 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -96,8 +96,8 @@ static int open_output_fd(char *path, const struct cache_entry *ce, int to_tempf
>  {
>  	int symlink = (ce->ce_mode & S_IFMT) != S_IFREG;
>  	if (to_tempfile) {
> -		strcpy(path, symlink
> -		       ? ".merge_link_XXXXXX" : ".merge_file_XXXXXX");
> +		xsnprintf(path, TEMPORARY_FILENAME_LENGTH, "%s",
> +			  symlink ? ".merge_link_XXXXXX" : ".merge_file_XXXXXX");
>  		return mkstemp(path);
>  	} else {
>  		return create_file(path, !symlink ? ce->ce_mode : 0666);

Hmm, I was going to suggest strlcpy() again. However, if you expect an overflow to
occur, then xsnprintf() will at least bring it to your attention!  Checking for overflow
with strlcpy() is not rocket science either, and I guess we could add xstrlcpy() ... :-D

dunno.

ATB,
Ramsay Jones