[PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Ramsay Jones]

The msvc compiler thinks that a variable could be used while
uninitialised and issues the following warning:

    ...\git\wt-status.c(152) : warning C4700: uninitialized local \
        variable 'status' used

In order to suppress the warning, we simply initialise the
variable to zero.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 wt-status.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index d9f3d9f..9b4b5bf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -149,7 +149,7 @@ static void wt_status_print_change_data(struct wt_status *s,
 {
 	struct wt_status_change_data *d = it->util;
 	const char *c = color(change_type, s);
-	int status = status;
+	int status = 0;
 	char *one_name;
 	char *two_name;
 	const char *one, *two;
-- 
1.7.3

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Jonathan Nieder]

Ramsay Jones wrote:

> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -149,7 +149,7 @@ static void wt_status_print_change_data(struct wt_status *s,
>  {
>  	struct wt_status_change_data *d = it->util;
>  	const char *c = color(change_type, s);
> -	int status = status;
> +	int status = 0;

Just for the record (I assume you are already aware of this):

 http://thread.gmane.org/gmane.comp.version-control.git/133278/focus=133422
 http://thread.gmane.org/gmane.comp.version-control.git/124676/focus=124803

I personally feel lukewarm about this kind of change.  Is it possible to
suppress this warning from msvc?

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Ramsay Jones]

Jonathan Nieder wrote:
> 
> Just for the record (I assume you are already aware of this):
> 
>  http://thread.gmane.org/gmane.comp.version-control.git/133278/focus=133422
>  http://thread.gmane.org/gmane.comp.version-control.git/124676/focus=124803

Yes

> I personally feel lukewarm about this kind of change.  

OK.

Junio, could you please drop patches 5-14 from the series; the first four patches
are the important ones and I'd rather they didn't get held up. Thanks!

ATB,
Ramsay Jones

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Junio, could you please drop patches 5-14 from the series; the first four patches
> are the important ones and I'd rather they didn't get held up. Thanks!

Have these four patches been Acked by interested parties?

I think I saw 1/N and 2/N acked by Erik and 4/N acked by SSchuberth and
J6t, but any words on 3/N?

Not that I deeply care nor have environment to test changes to [3/N], but
I am wondering if these need conditional definition to futureproof (e.g.
what happens when the header you are using the definition _I64_MIN from,
or some other headers, started defining these constats?).

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Erik Faye-Lund]

On Thu, Dec 9, 2010 at 8:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> Junio, could you please drop patches 5-14 from the series; the first four patches
>> are the important ones and I'd rather they didn't get held up. Thanks!
>
> Have these four patches been Acked by interested parties?
>
> I think I saw 1/N and 2/N acked by Erik and 4/N acked by SSchuberth and
> J6t, but any words on 3/N?
>
> Not that I deeply care nor have environment to test changes to [3/N], but
> I am wondering if these need conditional definition to futureproof (e.g.
> what happens when the header you are using the definition _I64_MIN from,
> or some other headers, started defining these constats?).

I'm not sure if I follow this entirely. _I64_MIN is defined by
limits.h on Windows, and limits.h has a header-guard (or "#pragma
once" as Microsoft-code tends to prefer).

Oh, right. You mean if someone else starts defining INTMAX_MAX etc? If
someone includes an stdint/inttypes-implementation while including
git-compat-util.h, we're going to have a boat-load of similar issues
anyway. I think guarding them is something that's better left to when
we encounter the problem (if ever).

All in all, patch 1 though 4 looks good to me. And thanks to Ramsay
for cleaning up my mess :)

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Sebastian Schuberth]

On Thu, Dec 9, 2010 at 20:46, Erik Faye-Lund <kusmabite@gmail.com> wrote:

> On Thu, Dec 9, 2010 at 8:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> Junio, could you please drop patches 5-14 from the series; the first four patches
>>> are the important ones and I'd rather they didn't get held up. Thanks!
>>
>> Have these four patches been Acked by interested parties?
>>
>> I think I saw 1/N and 2/N acked by Erik and 4/N acked by SSchuberth and
>> J6t, but any words on 3/N?
>>
>> Not that I deeply care nor have environment to test changes to [3/N], but
>> I am wondering if these need conditional definition to futureproof (e.g.
>> what happens when the header you are using the definition _I64_MIN from,
>> or some other headers, started defining these constats?).
>
> I'm not sure if I follow this entirely. _I64_MIN is defined by
> limits.h on Windows, and limits.h has a header-guard (or "#pragma
> once" as Microsoft-code tends to prefer).
>
> Oh, right. You mean if someone else starts defining INTMAX_MAX etc? If
> someone includes an stdint/inttypes-implementation while including
> git-compat-util.h, we're going to have a boat-load of similar issues
> anyway. I think guarding them is something that's better left to when
> we encounter the problem (if ever).

FYI: In contrast to previous versions, Visual Studio 2010 ships with a
stdint.h header which defines INTMAX_MAX etc. However, that stdint.h
is not included by limits.h (in fact, not by *any* other shipping
header file, as it seems), so we should not run into any trouble even
with VS2010.

So I agree with Erik about patch 3/N:

Acked-by: Sebastian Schuberth <sschuberth@gmail.com>

-- 
Sebastian Schuberth

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Erik Faye-Lund]

On Fri, Dec 10, 2010 at 2:35 PM, Sebastian Schuberth
<sschuberth@gmail.com> wrote:
> On Thu, Dec 9, 2010 at 20:46, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>
>> On Thu, Dec 9, 2010 at 8:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>>
>>>> Junio, could you please drop patches 5-14 from the series; the first four patches
>>>> are the important ones and I'd rather they didn't get held up. Thanks!
>>>
>>> Have these four patches been Acked by interested parties?
>>>
>>> I think I saw 1/N and 2/N acked by Erik and 4/N acked by SSchuberth and
>>> J6t, but any words on 3/N?
>>>
>>> Not that I deeply care nor have environment to test changes to [3/N], but
>>> I am wondering if these need conditional definition to futureproof (e.g.
>>> what happens when the header you are using the definition _I64_MIN from,
>>> or some other headers, started defining these constats?).
>>
>> I'm not sure if I follow this entirely. _I64_MIN is defined by
>> limits.h on Windows, and limits.h has a header-guard (or "#pragma
>> once" as Microsoft-code tends to prefer).
>>
>> Oh, right. You mean if someone else starts defining INTMAX_MAX etc? If
>> someone includes an stdint/inttypes-implementation while including
>> git-compat-util.h, we're going to have a boat-load of similar issues
>> anyway. I think guarding them is something that's better left to when
>> we encounter the problem (if ever).
>
> FYI: In contrast to previous versions, Visual Studio 2010 ships with a
> stdint.h header which defines INTMAX_MAX etc. However, that stdint.h
> is not included by limits.h (in fact, not by *any* other shipping
> header file, as it seems), so we should not run into any trouble even
> with VS2010.
>

Very interesting, thanks. Did you try to compile Git on VS2010? This
sounds like a reason for me to install VS2010 on one of my machines...
:)

Re: [PATCH 14/14] wt-status.c: Initialise variable to suppress msvc warning

[Sebastian Schuberth]

On Fri, Dec 10, 2010 at 16:05, Erik Faye-Lund <kusmabite@gmail.com> wrote:

>> FYI: In contrast to previous versions, Visual Studio 2010 ships with a
>> stdint.h header which defines INTMAX_MAX etc. However, that stdint.h
>> is not included by limits.h (in fact, not by *any* other shipping
>> header file, as it seems), so we should not run into any trouble even
>> with VS2010.
>>
>
> Very interesting, thanks. Did you try to compile Git on VS2010? This
> sounds like a reason for me to install VS2010 on one of my machines...
> :)

I just gave it a quick try by just importing the VS2008 solution, and
there seem to be some CRT related link problems, but these should be
easy to resolve. I don't have the time right now to look any further
into this, sorry.

-- 
Sebastian Schuberth