Subject: Re: [PATCH v0 3/3] Build in git-rebase.sh

From: Johannes Sixt <[email-masked]>
Date: Mon, 25 May 2009 08:16:13 +0200

Nguyen Thai Ngoc Duy schrieb:
> On Sat, May 23, 2009 at 07:26:03PM +1000, Nguyen Thai Ngoc Duy wrote:
>> On Fri, May 22, 2009 at 05:30:31PM +1000, Nguyen Thai Ngoc Duy wrote:
>>> 2009/5/22 Johannes Sixt <[email-masked]>:
>>>> Nguyễn Thái Ngọc Duy schrieb:
>>>>> Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
>>>> Is it possible for you to test this series on Windows? Many rebase tests
>>>> fail, but I haven't investigated why.
>>> I'll try it this weekend.
>> This patch makes t3*rebase*.sh pass for me except t3412 (more exactly
>> t3412.8). That test failed even with git-rebase.sh. Hmm... Anyway
>> could you try again to see what tests still fail?
> 
> Someone with better Windows knowledge than me should explain how this works. Windows'
> snprintf() just cuts out the last character in this statement:
> 
> snprintf(buf, 8, "--%s", "onto"); // result: '--ont', expected: '--onto'

This doesn't happen for me: neither with Windows's original snprintf nor
with the version from compat/. Are you using the latest msysgit
environment to compile, i.e. gcc 4.4? There was a change regarding
SNPRINTF_SIZE_CORR; perhaps that's the culprit?

I don't undertand what this patch does, anyway. Where is the detail that I
am missing?

> All rebase tests now pass for me on Windows (Vista something, I have
> yet to find where it hides its "uname" command)

They also pass for me with your earlier fix-up patch, but with or without
this patch to compat/snprintf.c.

> diff --git a/compat/snprintf.c b/compat/snprintf.c
> index 357e733..1cea768 100644
> --- a/compat/snprintf.c
> +++ b/compat/snprintf.c
> @@ -13,7 +13,7 @@
>  int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
>  {
>  	char *s;
> -	int ret = -1;
> +	int size, ret = -1;
>  
>  	if (maxsize > 0) {
>  		ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
> @@ -26,18 +26,19 @@ int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
>  		return ret;
>  
>  	s = NULL;
> -	if (maxsize < 128)
> -		maxsize = 128;
> +	size = maxsize < 128 ? 128 : maxsize;
>  
>  	while (ret == -1) {
> -		maxsize *= 4;
> -		str = realloc(s, maxsize);
> -		if (! str)
> +		size *= 4;
> +		s = realloc(s, size);
> +		if (!s)
>  			break;
> -		s = str;
> -		ret = vsnprintf(str, maxsize-SNPRINTF_SIZE_CORR, format, ap);
> -		if (ret == maxsize-1)
> +		s = s;
> +		ret = vsnprintf(s, size-SNPRINTF_SIZE_CORR, format, ap);
> +		if (ret == size-1)
>  			ret = -1;
> +		else
> +			memcpy(str, s, maxsize-1);
>  	}
>  	free(s);
>  	return ret;