threads / patch / 25957

patch, 14 partswt-status.c: Initialise variable to suppress msvc warning

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

## tl;dr

8 messages between Dec 4, 2010 and Dec 10, 2010. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Ramsay Jones· Dec 4, 2010, 19:12 UTC · lore

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(-)
Show changes to wt-status.c +1 −1
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
Jonathan Nieder· Dec 4, 2010, 20:52 UTC · re: Ramsay Jones · lore

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

Ramsay Jones wrote:
Show 8 quoted lines
> --- 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?

Ramsay Jones· Dec 9, 2010, 18:17 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder wrote:
Show 5 quoted lines
> 
> 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

Junio C Hamano· Dec 9, 2010, 19:08 UTC · re: Ramsay Jones · lore

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

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?).

Erik Faye-Lund· Dec 9, 2010, 19:46 UTC · re: Junio C Hamano · lore

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

On Thu, Dec 9, 2010 at 8:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 14 quoted lines
> 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 :)

Sebastian Schuberth· Dec 10, 2010, 13:35 UTC · re: Erik Faye-Lund · lore

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

On Thu, Dec 9, 2010 at 20:46, Erik Faye-Lund <kusmabite@gmail.com> wrote:
Show 25 quoted lines
> 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
Erik Faye-Lund· Dec 10, 2010, 15:05 UTC · re: Sebastian Schuberth · lore

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

On Fri, Dec 10, 2010 at 2:35 PM, Sebastian Schuberth <sschuberth@gmail.com> wrote:

Show 34 quoted lines
> 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... :)

Sebastian Schuberth· Dec 10, 2010, 15:43 UTC · re: Erik Faye-Lund · lore

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

On Fri, Dec 10, 2010 at 16:05, Erik Faye-Lund <kusmabite@gmail.com> wrote:
Show 10 quoted lines
>> 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

← back to recent threads