Subject: Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

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

On 15/09/15 19:42, Jeff King wrote:
> On Tue, Sep 15, 2015 at 07:32:29PM +0100, Ramsay Jones wrote:
>
>>> diff --git a/archive-tar.c b/archive-tar.c
>>> index b6b30bb..d543f93 100644
>>> --- a/archive-tar.c
>>> +++ b/archive-tar.c
>>> @@ -301,7 +301,7 @@ static int write_global_extended_header(struct archiver_args *args)
>>>  	memset(&header, 0, sizeof(header));
>>>  	*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
>>>  	mode = 0100666;
>>> -	strcpy(header.name, "pax_global_header");
>>> +	xsnprintf(header.name, sizeof(header.name), "pax_global_header");
>> How about using strlcpy() instead? Thus:
>>
>> -	strcpy(header.name, "pax_global_header");
>> +	strlcpy(header.name, "pax_global_header", sizeof(header.name));
>>
>> Ditto for other similar (strcpy->xsnprintf) hunks below.
> That misses the "assert" behavior of xsnprintf. We are preventing
> overflow here, but also truncation. What should happen if
> "pax_global_header" does not fit in header.name? I think complaining
> loudly and immediately is the most helpful thing, because it is surely a
> programming error.
>
> We could make xstrlcpy(), of course, but I don't see much point when
> xsnprintf does the same thing (and more).

Heh, I just sent an email about patch 22/67 which says similar things. I don't feel
too strongly, either way, but I have a slight preference for the use of [x]strlcpy()
in these cases.

I have to stop at patch #22 for now.

ATB,
Ramsay Jones