Breakage in master?

Breakage in master?

[Erik Faye-Lund]

Something strange is going on in Junio's current 'master' branch
(f3fb075). "git show" has started to error out on Windows with a
complaint about our vsnprintf:
---8<---

$ git show
commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
Author: Junio C Hamano <gitster@pobox.com>
Date:   Tue Jan 31 22:31:35 2012 -0800

    Update draft release notes to 1.7.10

    Signed-off-by: Junio C Hamano <gitster@pobox.com>

fatal: BUG: your vsnprintf is broken (returned -1)
---8<---

"git status" is also behaving strange:
---8<---
$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       ←[31mcompat/vcbuild/include/sys/resource.h←[m
#       ←[31mtemp.patch←[m
#       ←[31mtest.c←[m
#       ←[31mtest.patch←[m
nothing added to commit but untracked files present (use "git add" to track)
---8<---

Yeah, the ANSI color codes are being printed verbatim, even though
compat/winansi.c is supposed to convert these. "git -p status" works
fine, as it pipes the ANSI codes directly through less.

But here's the REALLY puzzling part: If I add a simple, unused
function to diff-lib.c, like this:

---8<---
diff --git a/diff-lib.c b/diff-lib.c
index fc0dff3..914a224 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -82,6 +82,11 @@ static int match_stat_with_submodule(struct
diff_options *diffopt,
 	return changed;
 }

+static unsigned int foo(const char *a, unsigned int b)
+{
+	return b;
+}
+
 int run_diff_files(struct rev_info *revs, unsigned int option)
 {
 	int entries, i;
---8<---

"git status" starts to error out with that same vsnprintf complaint!

---8<---
$ git status
# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
fatal: BUG: your vsnprintf is broken (returned -1)
---8<---

Here's the stack-trace:
---8<---
Breakpoint 1, die (err=0x5268ec "BUG: your vsnprintf is broken (returned %d)")
    at usage.c:81
81      void NORETURN die(const char *err, ...)
(gdb) bt
#0  die (err=0x5268ec "BUG: your vsnprintf is broken (returned %d)")
    at usage.c:81
#1  0x00466314 in strbuf_vaddf (sb=0x28fb54,
    fmt=0x533ef4 "  (use \"git checkout -- <file>...\" to discard changes in wor
king directory)", ap=0x28fbac "+\025\026\002") at strbuf.c:221
#2  0x004cbb6f in status_vprintf (s=0x28fc38, at_bol=0,
    color=0x46c4f2 "\205└t\n\203─\020[^╔├\215v",
    fmt=0x533ef4 "  (use \"git checkout -- <file>...\" to discard changes in wor
king directory)", ap=0x28fbac "+\025\026\002", trail=0x533a89 "\n")
    at wt-status.c:44
#3  0x004cbe6b in status_printf_ln (s=0x28fc38, color=0x28fc70 "",
    fmt=0x533ef4 "  (use \"git checkout -- <file>...\" to discard changes in wor
king directory)") at wt-status.c:87
#4  0x004cced1 in wt_status_print (s=0x28fc38) at wt-status.c:176
#5  0x0041922e in cmd_status (argc=1, argv=0x319b4, prefix=0x0)
    at builtin/commit.c:1254
#6  0x004019d6 in handle_internal_command (argc=<value optimized out>,
    argv=<value optimized out>) at git.c:308
#7  0x00401c26 in main (argc=2, argv=0x319b0) at git.c:513
(gdb)
---8<---

This smells a bit like a smashed stack to me. Both issues happens in
roughly the same area of the call stack, and when adding an unused
function changes behavior, something really odd is going on ;)

I've bisected the issues down to 5e9637c (i18n: add infrastructure for
translating Git with gettext). Trying to apply my unused-function
patch on top of this commit starts giving the same "fatal: BUG: your
vsnprintf is broken (returned -1)" error. It's ancestor, bc1bbe0(Git
1.7.8-rc2), does not yield any of the issues.

I'm at a loss here. Does anyone have a hunch about what's going on?

Re: Breakage in master?

[Jeff King]

On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:

> But here's the REALLY puzzling part: If I add a simple, unused
> function to diff-lib.c, like this:
> [...]
> "git status" starts to error out with that same vsnprintf complaint!
> 
> ---8<---
> $ git status
> # On branch master
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> fatal: BUG: your vsnprintf is broken (returned -1)
> ---8<---

OK, that's definitely odd.

At the moment of the die() in strbuf_vaddf, what does errno say?
vsnprintf should generally never be returning -1 (it should return the
number of characters that would have been written). Since you're on
Windows, I assume you're using the replacement version in
compat/snprintf.c.

That one will return -1 if realloc fails. So I'm curious if that is what
is happening (you might also instrument the call to realloc in
snprintf.c to see if it is failing, and if so, at what maxsize). And/or
check errno in git_vsnprintf after calling the native vsnprintf and
getting -1.

Here's one possible sequence of events that seems plausible to me (and
remember that this is a wild guess):

  1. gettext somehow munges the format string in a way that Windows
     vsnprintf doesn't like, and it returns -1.

  2. Our git_vsnprintf wrapper interprets this -1 as "you didn't give me
     enough space to store the result", and we grow our test-buffer to
     try again

  3. Eventually the test buffer gets unreasonably large, and realloc
     fails. We have no choice but to return -1 from our wrapper.

  4. strbuf_vaddf sees the -1 and thinks you are using a broken
     vsnprintf.

All of that would make sense to me, _except_ for your weird "if I add a
random function, the problem is more reproducible" bit. Which does seem
like something is invoking undefined behavior (of course, it could be
that undefined behavior or stack-smashing that is causing vsnprintf to
report an error). Lacking any better leads, it might be worth pursuing.

> I've bisected the issues down to 5e9637c (i18n: add infrastructure for
> translating Git with gettext). Trying to apply my unused-function
> patch on top of this commit starts giving the same "fatal: BUG: your
> vsnprintf is broken (returned -1)" error. It's ancestor, bc1bbe0(Git
> 1.7.8-rc2), does not yield any of the issues.

I've looked at 5e9637c, and it really doesn't do anything that looks
bad. I wonder if your gettext library is buggy. Does compiling with
NO_GETTEXT help?

-Peff

Re: [msysGit] Breakage in master?

[Johannes Schindelin]

Hi Erik,

On Thu, 2 Feb 2012, Erik Faye-Lund wrote:

> Something strange is going on in Junio's current 'master' branch
> (f3fb075). "git show" has started to error out on Windows with a
> complaint about our vsnprintf:
> ---8<---
> 
> $ git show
> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Tue Jan 31 22:31:35 2012 -0800
> 
>     Update draft release notes to 1.7.10
> 
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> fatal: BUG: your vsnprintf is broken (returned -1)
> ---8<---
>
> [...]
>
> I'm at a loss here. Does anyone have a hunch about what's going on?

It very much reminds me of 6ef404095bc1162031fc3cb43430b512e975bc6a...

Is it possible that NO_GETTEXT is either not set, or ignored?

Ciao,
Dscho

Re: [msysGit] Breakage in master?

[Torsten Bögershausen]

On 02.02.12 19:57, Johannes Schindelin wrote:
> Hi Erik,
> 
> On Thu, 2 Feb 2012, Erik Faye-Lund wrote:
> 
>> Something strange is going on in Junio's current 'master' branch
>> (f3fb075). "git show" has started to error out on Windows with a
>> complaint about our vsnprintf:
>> ---8<---
>>
>> $ git show
>> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
>> Author: Junio C Hamano <gitster@pobox.com>
>> Date:   Tue Jan 31 22:31:35 2012 -0800
>>
>>     Update draft release notes to 1.7.10
>>
>>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>
>> fatal: BUG: your vsnprintf is broken (returned -1)
>> ---8<---
>>
>> [...]
>>
>> I'm at a loss here. Does anyone have a hunch about what's going on?
> 
> It very much reminds me of 6ef404095bc1162031fc3cb43430b512e975bc6a...
> 
> Is it possible that NO_GETTEXT is either not set, or ignored?
> 
> Ciao,
> Dscho
Hi,
same problem here, tested on Win XP.

Good news (?):
Setting 
NO_GETTEXT=YesPlease
in the Makefile makes the problem go away for me.
/Torsten

Re: [msysGit] Breakage in master?

[Johannes Sixt]

Am 02.02.2012 13:14, schrieb Erik Faye-Lund:
> Something strange is going on in Junio's current 'master' branch
> (f3fb075). "git show" has started to error out on Windows with a
> complaint about our vsnprintf:
> ---8<---
> 
> $ git show
> commit f3fb07509c2e0b21b12a598fcd0a19a92fc38a9d
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Tue Jan 31 22:31:35 2012 -0800
> 
>     Update draft release notes to 1.7.10
> 
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> fatal: BUG: your vsnprintf is broken (returned -1)

FWIW, I run a version of master on Windows with almost no additional
patches that touch compat/ (the few changes I do have are unsuspicious).
But I do not observe this behavior. I build with NO_GETTEXT and -O0.

-- Hannes

Re: Breakage in master?

[Erik Faye-Lund]

On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:
>
>> But here's the REALLY puzzling part: If I add a simple, unused
>> function to diff-lib.c, like this:
>> [...]
>> "git status" starts to error out with that same vsnprintf complaint!
>>
>> ---8<---
>> $ git status
>> # On branch master
>> # Changes not staged for commit:
>> #   (use "git add <file>..." to update what will be committed)
>> fatal: BUG: your vsnprintf is broken (returned -1)
>> ---8<---
>
> OK, that's definitely odd.
>
> At the moment of the die() in strbuf_vaddf, what does errno say?

If I apply this patch:
---8<---
diff --git a/strbuf.c b/strbuf.c
index ff0b96b..52dfdd6 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char
*fmt, va_list ap)
 	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
 	va_end(cp);
 	if (len < 0)
-		die("BUG: your vsnprintf is broken (returned %d)", len);
+		die_errno("BUG: your vsnprintf is broken (returned %d)", len);
 	if (len > strbuf_avail(sb)) {
 		strbuf_grow(sb, len);
 		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
---8<---

Then I get "fatal: BUG: your vsnprintf is broken (returned -1): Result
too large". This goes both for both failure cases I described. I
assume this means errno=ERANGE.

> vsnprintf should generally never be returning -1 (it should return the
> number of characters that would have been written). Since you're on
> Windows, I assume you're using the replacement version in
> compat/snprintf.c.

No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
the MinGW target. I'm assuming that means MinGW-runtime has a sane
vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,
the problem occurs. And it's still "Result too large".

So I decided to do a bit of stepping, and it seems libintl takes over
vsnprintf, directing us to libintl_vsnprintf instead. I guess this is
so it can ensure we support reordering the parameters with $1 etc...
And aparently this vsnprintf implementation calls the system vnsprintf
if the format string does not contain '$', and it's using _vsnprintf
rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,
and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.

So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.

> All of that would make sense to me, _except_ for your weird "if I add a
> random function, the problem is more reproducible" bit. Which does seem
> like something is invoking undefined behavior (of course, it could be
> that undefined behavior or stack-smashing that is causing vsnprintf to
> report an error). Lacking any better leads, it might be worth pursuing.

Well, now at least I have some better leads, but I'm still not able to
explain the "if I add a random function, the problem is more
reproducible" bit. :(

>> I've bisected the issues down to 5e9637c (i18n: add infrastructure for
>> translating Git with gettext). Trying to apply my unused-function
>> patch on top of this commit starts giving the same "fatal: BUG: your
>> vsnprintf is broken (returned -1)" error. It's ancestor, bc1bbe0(Git
>> 1.7.8-rc2), does not yield any of the issues.
>
> I've looked at 5e9637c, and it really doesn't do anything that looks
> bad. I wonder if your gettext library is buggy. Does compiling with
> NO_GETTEXT help?

Compiling with NO_GETTEXT does make the symptoms go away, but that's
not curing the problem ;)

But, I have a lead now. I'll see if I can find out *why* libintl calls
_vsnprintf on MinGW. I expect it's so the MSVC and the MinGW versions
behave similarly, MSVC doesn't have a sane vsnprintf. Perhaps I should
back-port SNPRINTF_RETURNS_BOGUS-workaround to libintl, so our MSVC
builds doesn't break also?

Re: Breakage in master?

[Joel C. Salomon]

On 02/03/2012 07:28 AM, Erik Faye-Lund wrote:
> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
>> vsnprintf should generally never be returning -1 (it should return the
>> number of characters that would have been written). Since you're on
>> Windows, I assume you're using the replacement version in
>> compat/snprintf.c.
> 
> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
> the MinGW target. I'm assuming that means MinGW-runtime has a sane
> vsnprintf implementation.

That doesn't sound right; MinGW defaults to linking to a fairly old
version of the Windows C library (MSVCRT.dll from Visual Studio 6,
IIRC).  According to <http://mingw.org/wiki/C99> there exists libmingwex
with some functions (especially those from <stdio.h>) replaced for
Standard compatibility, but it's "far from complete".  (Is msysGit using
it anyway?)

--Joel

Re: Breakage in master?

[Erik Faye-Lund]

On Fri, Feb 3, 2012 at 2:55 PM, Joel C. Salomon <joelcsalomon@gmail.com> wrote:
> On 02/03/2012 07:28 AM, Erik Faye-Lund wrote:
>> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
>>> vsnprintf should generally never be returning -1 (it should return the
>>> number of characters that would have been written). Since you're on
>>> Windows, I assume you're using the replacement version in
>>> compat/snprintf.c.
>>
>> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
>> the MinGW target. I'm assuming that means MinGW-runtime has a sane
>> vsnprintf implementation.
>
> That doesn't sound right; MinGW defaults to linking to a fairly old
> version of the Windows C library (MSVCRT.dll from Visual Studio 6,
> IIRC).  According to <http://mingw.org/wiki/C99> there exists libmingwex
> with some functions (especially those from <stdio.h>) replaced for
> Standard compatibility, but it's "far from complete".  (Is msysGit using
> it anyway?)

I'm not entirely sure what you are arguing. On MinGW, calling
vsnprintf vs _vsnprintf leads to different implementations on MinGW.
This is documented in the release-notes:
http://sourceforge.net/project/shownotes.php?release_id=24832

"As in previous releases, the MinGW implementations of snprintf() and
vsnprintf() are the default for these two functions, with the MSVCRT
alternatives being called as _snprintf() and _vsnprintf()."

I don't see how this is contradicted by your argument of a third,
C99-ish implementation. I'm pretty sure the "far from complete"-part
is about the C99-features anyway.

Re: Breakage in master?

[Joel C. Salomon]

On 02/03/2012 09:05 AM, Erik Faye-Lund wrote:
> On Fri, Feb 3, 2012 at 2:55 PM, Joel C. Salomon <joelcsalomon@gmail.com> wrote:
>> That doesn't sound right; MinGW defaults to linking to a fairly old
>> version of the Windows C library (MSVCRT.dll from Visual Studio 6,
>> IIRC).
> 
> I'm not entirely sure what you are arguing. On MinGW, calling
> vsnprintf vs _vsnprintf leads to different implementations on MinGW.
> This is documented in the release-notes:
> http://sourceforge.net/project/shownotes.php?release_id=24832

Never mind; I stand corrected.  It's been some time since I actively
followed MinGW development.

--Joel

Re: Breakage in master?

[Erik Faye-Lund]

On Fri, Feb 3, 2012 at 1:28 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:
>>
>>> But here's the REALLY puzzling part: If I add a simple, unused
>>> function to diff-lib.c, like this:
>>> [...]
>>> "git status" starts to error out with that same vsnprintf complaint!
>>>
>>> ---8<---
>>> $ git status
>>> # On branch master
>>> # Changes not staged for commit:
>>> #   (use "git add <file>..." to update what will be committed)
>>> fatal: BUG: your vsnprintf is broken (returned -1)
>>> ---8<---
>>
>> OK, that's definitely odd.
>>
>> At the moment of the die() in strbuf_vaddf, what does errno say?
>
> If I apply this patch:
> ---8<---
> diff --git a/strbuf.c b/strbuf.c
> index ff0b96b..52dfdd6 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char
> *fmt, va_list ap)
>        len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
>        va_end(cp);
>        if (len < 0)
> -               die("BUG: your vsnprintf is broken (returned %d)", len);
> +               die_errno("BUG: your vsnprintf is broken (returned %d)", len);
>        if (len > strbuf_avail(sb)) {
>                strbuf_grow(sb, len);
>                len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> ---8<---
>
> Then I get "fatal: BUG: your vsnprintf is broken (returned -1): Result
> too large". This goes both for both failure cases I described. I
> assume this means errno=ERANGE.
>
>> vsnprintf should generally never be returning -1 (it should return the
>> number of characters that would have been written). Since you're on
>> Windows, I assume you're using the replacement version in
>> compat/snprintf.c.
>
> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
> the MinGW target. I'm assuming that means MinGW-runtime has a sane
> vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,
> the problem occurs. And it's still "Result too large".
>
> So I decided to do a bit of stepping, and it seems libintl takes over
> vsnprintf, directing us to libintl_vsnprintf instead. I guess this is
> so it can ensure we support reordering the parameters with $1 etc...
> And aparently this vsnprintf implementation calls the system vnsprintf
> if the format string does not contain '$', and it's using _vsnprintf
> rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,
> and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.
>
> So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.
>

Indeed, I just got around to testing this, and doing this on top of
gettext seems to fix the problem for me. For the MSVC, a more
elaborate fix is needed, as it doesn't have a sane vsnprintf.

---

diff --git a/gettext-runtime/intl/printf.c b/gettext-runtime/intl/printf.c
index b7cdc5d..f55023e 100644
--- a/gettext-runtime/intl/printf.c
+++ b/gettext-runtime/intl/printf.c
@@ -192,7 +192,7 @@ libintl_sprintf (char *resultbuf, const char *format, ...)

 #if HAVE_SNPRINTF

-# if HAVE_DECL__SNPRINTF
+# if HAVE_DECL__SNPRINTF && !defined(__MINGW32__)
    /* Windows.  */
 #  define system_vsnprintf _vsnprintf
 # else

Fwd: Breakage in master?

[Erik Faye-Lund]

Git has recently switched to using gettext for translations, and I
have observed a breakage on Windows due to the way gettext handles
vsnprintf.

On MinGW, vsnprintf and _vsnprintf are two different implementations;
vsnprintf is from MinGW-runtime, and provides a reasonably sane
implementation. _vnsprintf on the other hand is from MSVCRT.dll, and
has some issues with it's return value. Before using gettext, the
MinGW-built version of Git called the version from mingw-runtime, and
everything worked fine. When built with MSVC, a shim was used to fixup
the bogus return value. This shim was injected through a define,
similar to what gettext does.

The shim in gettext lead to issues for Git, both on MinGW and on MSVC.

For MinGW, the problem is that libintl_vsnprintf calls _vsnprintf
rather than vsnprintf, giving us the same, broken return value that we
tried to prevent. This means that our code intended to call the
MinGW-runtime version, but gettext ended up calling the MSVCRT.dll
version. I don't find this very reasonable; a call to vsnprintf ends
up as a call to _vsnprintf.

On MSVC the problem is a bit easier to spot; libgnuintl.h.in contains
the following:

---8<---
#if !(defined vsnprintf && defined _GL_STDIO_H) /* don't override gnulib */
#undef vsnprintf
#define vsnprintf libintl_vsnprintf
extern int vsnprintf (char *, size_t, const char *, va_list);
#endif
---8<---

Uhm, what? Unless we're using Gnulib, our definition of vsnprintf
should simply be ignored?

I'm not saying figuring out what to do here is exactly trivial; but I
think undefining any definitions of vsnprintf that aren't exactly
"_vsnprintf" is dangerous. The forwarded mail below contains a
quick-fix I did locally that seems to side-step the problem for me, by
not using _vsnprintf on MinGW. But perhaps there's something better we
can do?

---------- Forwarded message ----------
From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Sat, Feb 4, 2012 at 10:55 PM
Subject: Re: Breakage in master?
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>, msysGit
<msysgit@googlegroups.com>, Ævar Arnfjörð <avarab@gmail.com>


On Fri, Feb 3, 2012 at 1:28 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@peff.net> wrote:
>> On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:
>>
>>> But here's the REALLY puzzling part: If I add a simple, unused
>>> function to diff-lib.c, like this:
>>> [...]
>>> "git status" starts to error out with that same vsnprintf complaint!
>>>
>>> ---8<---
>>> $ git status
>>> # On branch master
>>> # Changes not staged for commit:
>>> #   (use "git add <file>..." to update what will be committed)
>>> fatal: BUG: your vsnprintf is broken (returned -1)
>>> ---8<---
>>
>> OK, that's definitely odd.
>>
>> At the moment of the die() in strbuf_vaddf, what does errno say?
>
> If I apply this patch:
> ---8<---
> diff --git a/strbuf.c b/strbuf.c
> index ff0b96b..52dfdd6 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char
> *fmt, va_list ap)
>        len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
>        va_end(cp);
>        if (len < 0)
> -               die("BUG: your vsnprintf is broken (returned %d)", len);
> +               die_errno("BUG: your vsnprintf is broken (returned %d)", len);
>        if (len > strbuf_avail(sb)) {
>                strbuf_grow(sb, len);
>                len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
> ---8<---
>
> Then I get "fatal: BUG: your vsnprintf is broken (returned -1): Result
> too large". This goes both for both failure cases I described. I
> assume this means errno=ERANGE.
>
>> vsnprintf should generally never be returning -1 (it should return the
>> number of characters that would have been written). Since you're on
>> Windows, I assume you're using the replacement version in
>> compat/snprintf.c.
>
> No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
> the MinGW target. I'm assuming that means MinGW-runtime has a sane
> vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,
> the problem occurs. And it's still "Result too large".
>
> So I decided to do a bit of stepping, and it seems libintl takes over
> vsnprintf, directing us to libintl_vsnprintf instead. I guess this is
> so it can ensure we support reordering the parameters with $1 etc...
> And aparently this vsnprintf implementation calls the system vnsprintf
> if the format string does not contain '$', and it's using _vsnprintf
> rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,
> and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.
>
> So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.
>

Indeed, I just got around to testing this, and doing this on top of
gettext seems to fix the problem for me. For the MSVC, a more
elaborate fix is needed, as it doesn't have a sane vsnprintf.

---

diff --git a/gettext-runtime/intl/printf.c b/gettext-runtime/intl/printf.c
index b7cdc5d..f55023e 100644
--- a/gettext-runtime/intl/printf.c
+++ b/gettext-runtime/intl/printf.c
@@ -192,7 +192,7 @@ libintl_sprintf (char *resultbuf, const char *format, ...)

 #if HAVE_SNPRINTF

-# if HAVE_DECL__SNPRINTF
+# if HAVE_DECL__SNPRINTF && !defined(__MINGW32__)
   /* Windows.  */
 #  define system_vsnprintf _vsnprintf
 # else

Re: Breakage in master?

[svnpenn]

When I use NO_GETTEXT=YesPlease I lose text coloring. I assume this is expected. 

Is there a workaround that doesnt lose text coloring?