[PATCH 0/3] MSVC fixes

[PATCH 0/3] MSVC fixes

[Sven Strickroth]

Hi,

I've 3 patches fixing warnings and errors when compiling with latest
MSVC (2012).

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

[PATCH 1/3] poll.h lies in the same folder, so use normal quotes for include

[Sven Strickroth]

Some compilers, like Visual C++ complain when <> is used instead of
double quotes for non system includes.

Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 compat/poll/poll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index 7d226ec..b85386a 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -31,7 +31,7 @@
 #include <sys/types.h>
 
 /* Specification.  */
-#include <poll.h>
+#include "poll.h"
 
 #include <errno.h>
 #include <limits.h>
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

[PATCH 2/3] mingw.h: Define only if necessary

[Sven Strickroth]

Since the latest version of MSVC EWOULDBLOCK, EAFNOSUPPORT and
ECONNABORTED are defined in errno.h. When used with MSVC mingw.h
is included from msvc.h and causes warnings about redefinitions.

Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 compat/mingw.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/compat/mingw.h b/compat/mingw.h
index 685cd2c..c424333 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -32,7 +32,9 @@ typedef int socklen_t;
 #define WEXITSTATUS(x) ((x) & 0xff)
 #define WTERMSIG(x) SIGTERM
 
+#ifndef EWOULDBLOCK
 #define EWOULDBLOCK EAGAIN
+#endif
 #define SHUT_WR SD_SEND
 
 #define SIGHUP 1
@@ -46,8 +48,12 @@ typedef int socklen_t;
 #define F_SETFD 2
 #define FD_CLOEXEC 0x1
 
+#ifndef EAFNOSUPPORT
 #define EAFNOSUPPORT WSAEAFNOSUPPORT
+#endif
+#ifndef ECONNABORTED
 #define ECONNABORTED WSAECONNABORTED
+#endif
 
 struct passwd {
 	char *pw_name;
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

[PATCH 3/3] Initialize variables with values

[Sven Strickroth]

With MSVC initializing a variable with "int a=a" causes a warning about
using an uninitialized value.

Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
 builtin/rev-list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 67701be..13afacd 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -338,7 +338,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		mark_edges_uninteresting(revs.commits, &revs, show_edge);
 
 	if (bisect_list) {
-		int reaches = reaches, all = all;
+		int reaches = 0, all = 0;
 
 		revs.commits = find_bisection(revs.commits, &reaches, &all,
 					      bisect_find_all);
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

Re: [PATCH 1/3] poll.h lies in the same folder, so use normal quotes for include

[Sven Strickroth]

Am 09.05.2013 03:12 schrieb Sven Strickroth:
> Some compilers, like Visual C++ complain when <> is used instead of
> double quotes for non system includes.

I just noticed that this patch isn't necessary for 1.8.3 (since
41f2999180f5a58f2a4214d896359c1587c9024f) any more. Sorry for the noise
- I was still building against 1.8.2.2.

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

Re: [PATCH 3/3] Initialize variables with values

[Krzysztof Mazur]

On Thu, May 09, 2013 at 03:13:39AM +0200, Sven Strickroth wrote:
> With MSVC initializing a variable with "int a=a" causes a warning about
> using an uninitialized value.
> 
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---
>  builtin/rev-list.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 67701be..13afacd 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -338,7 +338,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  		mark_edges_uninteresting(revs.commits, &revs, show_edge);
>  
>  	if (bisect_list) {
> -		int reaches = reaches, all = all;
> +		int reaches = 0, all = 0;
>  
>  		revs.commits = find_bisection(revs.commits, &reaches, &all,
>  					      bisect_find_all);

But this generates worse code, at least using gcc 4.7.2:

--- old	2013-05-09 14:33:22.000000000 +0200
+++ new	2013-05-09 14:33:02.000000000 +0200
@@ -1,2 +1,2 @@
    text	   data	    bss	    dec	    hex	filename
-   4283	      0	      0	   4283	   10bb	builtin/rev-list.o
+   4299	      0	      0	   4299	   10cb	builtin/rev-list.o

Krzysiek

Re: [PATCH 3/3] Initialize variables with values

[Jonathan Nieder]

Hi,

Sven Strickroth wrote:

> With MSVC initializing a variable with "int a=a" causes a warning about
> using an uninitialized value.
[...]
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -338,7 +338,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>  		mark_edges_uninteresting(revs.commits, &revs, show_edge);
>  
>  	if (bisect_list) {
> -		int reaches = reaches, all = all;
> +		int reaches = 0, all = 0;

A correct way to spell this is

		int reaches, all;

which, as a bonus, lets the compiler warn if they are used
uninitialized.  Does that provoke warnings?

Thanks,
Jonathan

Re: [PATCH 3/3] Initialize variables with values

[Sven Strickroth]

Am 09.05.2013 15:21 schrieb Jonathan Nieder:
> Sven Strickroth wrote:
> 
>> With MSVC initializing a variable with "int a=a" causes a warning about
>> using an uninitialized value.
> [...]
>> --- a/builtin/rev-list.c
>> +++ b/builtin/rev-list.c
>> @@ -338,7 +338,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>>  		mark_edges_uninteresting(revs.commits, &revs, show_edge);
>>  
>>  	if (bisect_list) {
>> -		int reaches = reaches, all = all;
>> +		int reaches = 0, all = 0;
> 
> A correct way to spell this is
> 
> 		int reaches, all;
> 
> which, as a bonus, lets the compiler warn if they are used
> uninitialized.  Does that provoke warnings?

This seems to be ok.

-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server

Re: [PATCH 3/3] Initialize variables with values

[René Scharfe]

Am 09.05.2013 15:21, schrieb Jonathan Nieder:
> Hi,
>
> Sven Strickroth wrote:
>
>> With MSVC initializing a variable with "int a=a" causes a warning about
>> using an uninitialized value.
> [...]
>> --- a/builtin/rev-list.c
>> +++ b/builtin/rev-list.c
>> @@ -338,7 +338,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
>>   		mark_edges_uninteresting(revs.commits, &revs, show_edge);
>>
>>   	if (bisect_list) {
>> -		int reaches = reaches, all = all;
>> +		int reaches = 0, all = 0;
>
> A correct way to spell this is
>
> 		int reaches, all;
>
> which, as a bonus, lets the compiler warn if they are used
> uninitialized.  Does that provoke warnings?

Only find_bisection() (defined in bisect.c) is used to set these 
variables in that block.  While it sets "all" unconditionally, it 
doesn't always set "reaches" -- only if it actually finds something. 
That's still safe because the following code path errors out early if 
nothing was found before it uses "reaches".

Are there C compilers that can analyse initialization and usage of 
variables across compilation units like that?

Anyway, initializing the variables to zero makes this code consistent 
with the second call-site of find_bisection().  Making sure this 
function sets "reaches" unconditionally as well and dropping the 
initialization from both places may be even better.

René