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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
