Re: git segfaults on older Solaris releases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

>> So perhaps this is all we need to fix your box.
>> 
>>  setup.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/setup.c b/setup.c
>> index 3439ec6..b6c8aab 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -103,7 +103,7 @@ char *prefix_path_gently(const char *prefix, int len,
>>  			return NULL;
>>  		}
>>  	} else {
>> -		sanitized = xstrfmt("%.*s%s", len, prefix, path);
>> +		sanitized = xstrfmt("%.*s%s", len, prefix ? prefix : "", path);
>>  		if (remaining_prefix)
>>  			*remaining_prefix = len;
>>  		if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix)) {
>
> The original pre-75faa45ae0230b321bf72027b2274315d7e14e34 version
> checked "if (len)", but I think this should be equally right.

That's a good point.  The original had

-               sanitized = xmalloc(len + strlen(path) + 1);
-               if (len)
-                       memcpy(sanitized, prefix, len);
-               strcpy(sanitized + len, path);
+               sanitized = xstrfmt("%.*s%s", len, prefix, path);

and for a brief moment I was wondering why the strlen() did not
barf, until I realized that was on path not prefix.

-- >8 --
Subject: setup.c: do not feed NULL to "%.*s" even with the precision 0

A recent update 75faa45a (replace trivial malloc + sprintf / strcpy
calls with xstrfmt, 2015-09-24) rewrote

	prepare an empty buffer
	if (len)
        	append the first len bytes of "prefix" to the buffer
	append "path" to the buffer

that computed "path", optionally prefixed by "prefix", into

	xstrfmt("%.*s%s", len, prefix, path);

However, passing a NULL pointer to the printf(3) family of functions
to format it with %s conversion, even with the precision 0, i.e.

	xstrfmt("%.*s", 0, NULL)

yields undefined results, at least on some platforms.  

Avoid this problem by substituting prefix with "" when len==0, as
prefix can legally be NULL in that case.  This would mimick the
intent of the original code better.

Reported-by: "Tom G. Christensen" <tgc@xxxxxxxxxxxxxxx>
Helped-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.c b/setup.c
index 3439ec6..b4a92fe 100644
--- a/setup.c
+++ b/setup.c
@@ -103,7 +103,7 @@ char *prefix_path_gently(const char *prefix, int len,
 			return NULL;
 		}
 	} else {
-		sanitized = xstrfmt("%.*s%s", len, prefix, path);
+		sanitized = xstrfmt("%.*s%s", len, len ? prefix : "", path);
 		if (remaining_prefix)
 			*remaining_prefix = len;
 		if (normalize_path_copy_len(sanitized, sanitized, remaining_prefix)) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]