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

On 15/09/15 19:42, Jeff King wrote:

On Tue, Sep 15, 2015 at 07:32:29PM +0100, Ramsay Jones wrote:

quoted

quoted

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