[PATCH 00/11] Various msysgit patches

[PATCH 00/11] Various msysgit patches

[Steffen Prohaska]

Hello,
Here are more patches that are in msysgit but not yet in official
git.git.  The patches have been discussed in the following two
threads:

    http://thread.gmane.org/gmane.comp.version-control.msysgit/5373
    http://thread.gmane.org/gmane.comp.version-control.msysgit/4876

The first couple of patches are small an obvious.  The patches with
larger numbers are more complex and might need discussion.

Best,
Steffen

Edward Z. Yang (1):
  connect.c: Support PuTTY plink and TortoisePlink as SSH on Windows

Johannes Schindelin (6):
  MinGW: GCC >= 4 does not need SNPRINTF_SIZE_CORR anymore
  Quiet make: do not leave Windows behind
  Work around a regression in Windows 7, causing erase_in_line() to
    crash sometimes
  test-chmtime: work around Windows limitation
  winansi: fix compile warnings
  Fix warnings in nedmalloc when compiling with GCC 4.4.0

Marius Storm-Olsen (2):
  MinGW readdir reimplementation to support d_type
  Add custom memory allocator to MinGW and MacOS builds

Pat Thoyts (1):
  git: browsing paths with spaces when using the start command

Steffen Prohaska (1):
  MinGW: Teach Makefile to detect msysgit and apply specific settings

 Makefile                     |   26 +-
 compat/mingw.c               |   59 +
 compat/mingw.h               |   29 +
 compat/nedmalloc/License.txt |   23 +
 compat/nedmalloc/Readme.txt  |  136 +
 compat/nedmalloc/malloc.c.h  | 5752 ++++++++++++++++++++++++++++++++++++++++++
 compat/nedmalloc/nedmalloc.c |  966 +++++++
 compat/nedmalloc/nedmalloc.h |  180 ++
 compat/snprintf.c            |    4 +
 compat/win32.h               |    1 +
 compat/winansi.c             |    3 +-
 connect.c                    |    8 +-
 git-web--browse.sh           |    5 +-
 test-chmtime.c               |    9 +
 14 files changed, 7193 insertions(+), 8 deletions(-)
 create mode 100644 compat/nedmalloc/License.txt
 create mode 100644 compat/nedmalloc/Readme.txt
 create mode 100644 compat/nedmalloc/malloc.c.h
 create mode 100644 compat/nedmalloc/nedmalloc.c
 create mode 100644 compat/nedmalloc/nedmalloc.h

[PATCH 01/11] MinGW: GCC >= 4 does not need SNPRINTF_SIZE_CORR anymore

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Makefile          |    1 -
 compat/snprintf.c |    4 ++++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index eaae45d..77d8d9c 100644
--- a/Makefile
+++ b/Makefile
@@ -845,7 +845,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	UNRELIABLE_FSTAT = UnfortunatelyYes
 	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch
-	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/regex/regex.o compat/winansi.o
 	EXTLIBS += -lws2_32
diff --git a/compat/snprintf.c b/compat/snprintf.c
index 357e733..6c0fb05 100644
--- a/compat/snprintf.c
+++ b/compat/snprintf.c
@@ -6,8 +6,12 @@
  * number of characters to write without the trailing NUL.
  */
 #ifndef SNPRINTF_SIZE_CORR
+#if defined(__MINGW32__) && defined(__GNUC__) && __GNUC__ < 4
+#define SNPRINTF_SIZE_CORR 1
+#else
 #define SNPRINTF_SIZE_CORR 0
 #endif
+#endif
 
 #undef vsnprintf
 int git_vsnprintf(char *str, size_t maxsize, const char *format, va_list ap)
-- 
1.6.3.1.54.g99dd

[PATCH 02/11] Quiet make: do not leave Windows behind

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

On Windows, we have to check whether there are scripts which would
override .exe files, but this check missed the "quietification".
Make now prints 'BUILTIN all' instead of a long chain of 'test || rm'
commands.

[spr: added clarification what make will print. ]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 77d8d9c..af08257 100644
--- a/Makefile
+++ b/Makefile
@@ -1202,7 +1202,7 @@ SHELL = $(SHELL_PATH)
 
 all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
-	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test '$p' -ef '$p$X' || $(RM) '$p';)
+	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test '$p' -ef '$p$X' || $(RM) '$p';)
 endif
 
 all::
-- 
1.6.3.1.54.g99dd

[PATCH 03/11] Work around a regression in Windows 7, causing erase_in_line() to crash sometimes

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The function FillConsoleOutputCharacterA() was pretty content in XP to take a NULL
pointer if we did not want to store the number of written columns.  In Windows 7,
it crashes, but only when called from within Git Bash, not from within cmd.exe.
Go figure.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/winansi.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index 44dc293..4bee335 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -80,6 +80,7 @@ static void set_console_attr(void)
 static void erase_in_line(void)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
+	long dummy; /* Needed for Windows 7 (or Vista) regression */
 
 	if (!console)
 		return;
@@ -87,7 +88,7 @@ static void erase_in_line(void)
 	GetConsoleScreenBufferInfo(console, &sbi);
 	FillConsoleOutputCharacterA(console, ' ',
 		sbi.dwSize.X - sbi.dwCursorPosition.X, sbi.dwCursorPosition,
-		NULL);
+		&dummy);
 }
 
 
-- 
1.6.3.1.54.g99dd

[PATCH 04/11] test-chmtime: work around Windows limitation

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Windows has problems changing the mtime when the file is write protected,
even by the owner of said file.

Add a Windows-only workaround to change the mode if necessary before
trying to change the mtime.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 test-chmtime.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/test-chmtime.c b/test-chmtime.c
index d5358cb..fe476cb 100644
--- a/test-chmtime.c
+++ b/test-chmtime.c
@@ -87,6 +87,15 @@ int main(int argc, const char *argv[])
 			return -1;
 		}
 
+#ifdef WIN32
+		if (!(sb.st_mode & S_IWUSR) &&
+				chmod(argv[i], sb.st_mode | S_IWUSR)) {
+			fprintf(stderr, "Could not make user-writable %s: %s",
+				argv[i], strerror(errno));
+			return -1;
+		}
+#endif
+
 		utb.actime = sb.st_atime;
 		utb.modtime = set_eq ? set_time : sb.st_mtime + set_time;
 
-- 
1.6.3.1.54.g99dd

[PATCH 05/11] winansi: fix compile warnings

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/win32.h   |    1 +
 compat/winansi.c |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/compat/win32.h b/compat/win32.h
index c26384e..d531130 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -1,5 +1,6 @@
 /* common Win32 functions for MinGW and Cygwin */
 #include <windows.h>
+#include <conio.h>
 
 static inline int file_attr_to_st_mode (DWORD attr)
 {
diff --git a/compat/winansi.c b/compat/winansi.c
index 4bee335..9217c24 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -80,7 +80,7 @@ static void set_console_attr(void)
 static void erase_in_line(void)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
-	long dummy; /* Needed for Windows 7 (or Vista) regression */
+	DWORD dummy; /* Needed for Windows 7 (or Vista) regression */
 
 	if (!console)
 		return;
-- 
1.6.3.1.54.g99dd

[PATCH 06/11] git: browsing paths with spaces when using the start command

[Steffen Prohaska]

From: Pat Thoyts <patthoyts@users.sourceforge.net>

msysGit issue 258 tracks a problem opening a browser onto file
paths that contain spaces or parentheses when calling the
web--browse script. This patch modifies how the start command is
called to solve this.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 git-web--browse.sh |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/git-web--browse.sh b/git-web--browse.sh
index 7ed0fad..4f5c740 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -161,9 +161,12 @@ case "$browser" in
 		;;
 	esac
 	;;
-    w3m|links|lynx|open|start)
+    w3m|links|lynx|open)
 	eval "$browser_path" "$@"
 	;;
+    start)
+        exec "$browser_path" '"web-browse"' "$@"
+        ;;
     dillo)
 	"$browser_path" "$@" &
 	;;
-- 
1.6.3.1.54.g99dd

[PATCH 07/11] connect.c: Support PuTTY plink and TortoisePlink as SSH on Windows

[Steffen Prohaska]

From: Edward Z. Yang <edwardzyang@thewritingpot.com>

OpenSSH uses -p to specify custom ports, while PuTTY plink and
TortoisePlink use -P. Git now detects if plink is in GIT_SSH and
modify its flags as necessary.

We call plink with -batch, so that it will error out with an error
message instead of waiting for user input.  As reported in msysGit
issue 96, plink wants to interact with the user asking if a host
key should be accepted, but this just blocks the terminal, since
plink tries to get the answer from stdin.  However, stdin is
already connected to Git that wants to send input to the remote
command.

But we do not pass -batch to TortoisePlink, because TortoisePlink
uses a GUI to communicate with the user, and it does not understand
-batch.

Signed-off-by: Edward Z. Yang <edwardzyang@thewritingpot.com>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 connect.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index f6b8ba6..692d476 100644
--- a/connect.c
+++ b/connect.c
@@ -602,14 +602,18 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 		die("command line too long");
 
 	conn->in = conn->out = -1;
-	conn->argv = arg = xcalloc(6, sizeof(*arg));
+	conn->argv = arg = xcalloc(7, sizeof(*arg));
 	if (protocol == PROTO_SSH) {
 		const char *ssh = getenv("GIT_SSH");
+		int putty = ssh && strcasestr(ssh, "plink");
 		if (!ssh) ssh = "ssh";
 
 		*arg++ = ssh;
+		if (putty && !strcasestr(ssh, "tortoiseplink"))
+			*arg++ = "-batch";
 		if (port) {
-			*arg++ = "-p";
+			/* P is for PuTTY, p is for OpenSSH */
+			*arg++ = putty ? "-P" : "-p";
 			*arg++ = port;
 		}
 		*arg++ = host;
-- 
1.6.3.1.54.g99dd

[PATCH 08/11] MinGW readdir reimplementation to support d_type

[Steffen Prohaska]

From: Marius Storm-Olsen <marius@trolltech.com>

The original readdir implementation was fast, but didn't
support the d_type. This means that git would do additional
lstats for each entry, to figure out if the entry was a
directory or not. This unneedingly slowed down many
operations, since Windows API provides this information
directly when walking the directories.

By running this implementation on Moe's repo structure:
  mkdir bummer && cd bummer; for ((i=0;i<100;i++)); do
    mkdir $i && pushd $i;
      for ((j=0;j<1000;j++)); do echo "$j" >$j; done;
    popd;
  done

We see the following speedups:
  git add .
  -------------------
  old: 00:00:23(.087)
  new: 00:00:21(.512) 1.07x

  git status
  -------------------
  old: 00:00:03(.306)
  new: 00:00:01(.684) 1.96x

  git clean -dxf
  -------------------
  old: 00:00:01(.918)
  new: 00:00:00(.295) 6.50x

Signed-off-by: Marius Storm-Olsen <marius@trolltech.com>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/mingw.c |   59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 compat/mingw.h |   29 +++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index e190fdd..d85d680 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1171,3 +1171,62 @@ char *getpass(const char *prompt)
 	fputs("\n", stderr);
 	return strbuf_detach(&buf, NULL);
 }
+
+#ifndef NO_MINGW_REPLACE_READDIR
+/* MinGW readdir implementation to avoid extra lstats for Git */
+struct mingw_DIR
+{
+	struct _finddata_t	dd_dta;		/* disk transfer area for this dir */
+	struct mingw_dirent	dd_dir;		/* Our own implementation, including d_type */
+	long			dd_handle;	/* _findnext handle */
+	int			dd_stat; 	/* 0 = next entry to read is first entry, -1 = off the end, positive = 0 based index of next entry */
+	char			dd_name[1]; 	/* given path for dir with search pattern (struct is extended) */
+};
+
+struct dirent *mingw_readdir(DIR *dir)
+{
+	WIN32_FIND_DATAA buf;
+	HANDLE handle;
+	struct mingw_DIR *mdir = (struct mingw_DIR*)dir;
+
+	if (!dir->dd_handle) {
+		errno = EBADF; /* No set_errno for mingw */
+		return NULL;
+	}
+
+	if (dir->dd_handle == (long)INVALID_HANDLE_VALUE && dir->dd_stat == 0)
+	{
+		handle = FindFirstFileA(dir->dd_name, &buf);
+		DWORD lasterr = GetLastError();
+		dir->dd_handle = (long)handle;
+		if (handle == INVALID_HANDLE_VALUE && (lasterr != ERROR_NO_MORE_FILES)) {
+			errno = err_win_to_posix(lasterr);
+			return NULL;
+		}
+	} else if (dir->dd_handle == (long)INVALID_HANDLE_VALUE) {
+		return NULL;
+	} else if (!FindNextFileA((HANDLE)dir->dd_handle, &buf)) {
+		DWORD lasterr = GetLastError();
+		FindClose((HANDLE)dir->dd_handle);
+		dir->dd_handle = (long)INVALID_HANDLE_VALUE;
+		/* POSIX says you shouldn't set errno when readdir can't
+		   find any more files; so, if another error we leave it set. */
+		if (lasterr != ERROR_NO_MORE_FILES)
+			errno = err_win_to_posix(lasterr);
+		return NULL;
+	}
+
+	/* We get here if `buf' contains valid data.  */
+	strcpy(dir->dd_dir.d_name, buf.cFileName);
+	++dir->dd_stat;
+
+	/* Set file type, based on WIN32_FIND_DATA */
+	mdir->dd_dir.d_type = 0;
+	if (buf.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+		mdir->dd_dir.d_type |= DT_DIR;
+	else
+		mdir->dd_dir.d_type |= DT_REG;
+
+	return (struct dirent*)&dir->dd_dir;
+}
+#endif // !NO_MINGW_REPLACE_READDIR
diff --git a/compat/mingw.h b/compat/mingw.h
index 4c50f5b..4f7ba4c 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -235,3 +235,32 @@ int main(int argc, const char **argv) \
 	return mingw_main(argc, argv); \
 } \
 static int mingw_main(c,v)
+
+#ifndef NO_MINGW_REPLACE_READDIR
+/*
+ * A replacement of readdir, to ensure that it reads the file type at
+ * the same time. This avoid extra unneeded lstats in git on MinGW
+ */
+#undef DT_UNKNOWN
+#undef DT_DIR
+#undef DT_REG
+#undef DT_LNK
+#define DT_UNKNOWN	0
+#define DT_DIR		1
+#define DT_REG		2
+#define DT_LNK		3
+
+struct mingw_dirent
+{
+	long		d_ino;			/* Always zero. */
+	union {
+		unsigned short	d_reclen;	/* Always zero. */
+		unsigned char   d_type;		/* Reimplementation adds this */
+	};
+	unsigned short	d_namlen;		/* Length of name in d_name. */
+	char		d_name[FILENAME_MAX];	/* File name. */
+};
+#define dirent mingw_dirent
+#define readdir(x) mingw_readdir(x)
+struct dirent *mingw_readdir(DIR *dir);
+#endif // !NO_MINGW_REPLACE_READDIR
-- 
1.6.3.1.54.g99dd

[PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/nedmalloc/malloc.c.h  |    4 +++-
 compat/nedmalloc/nedmalloc.c |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
index 1d9bbe0..678beb8 100644
--- a/compat/nedmalloc/malloc.c.h
+++ b/compat/nedmalloc/malloc.c.h
@@ -1270,7 +1270,9 @@ int mspace_mallopt(int, int);
 /*------------------------------ internal #includes ---------------------- */
 
 #ifdef WIN32
+#ifndef __GNUC__
 #pragma warning( disable : 4146 ) /* no "unsigned" warnings */
+#endif
 #endif /* WIN32 */
 
 #include <stdio.h>       /* for printing in malloc_stats */
@@ -2541,7 +2543,7 @@ struct malloc_params {
 static struct malloc_params mparams;
 
 /* Ensure mparams initialized */
-#define ensure_initialization() (mparams.magic != 0 || init_mparams())
+#define ensure_initialization() if (mparams.magic == 0) init_mparams()
 
 #if !ONLY_MSPACES
 
diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index a381a7d..60a4093 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
 /*#define FULLSANITYCHECKS*/
 
 #include "nedmalloc.h"
-#if defined(WIN32) && !defined(__MINGW32__)
+#if defined(WIN32)
  #include <malloc.h>
 #endif
 #define MSPACES 1
-- 
1.6.3.1.54.g99dd

[PATCH 11/11] MinGW: Teach Makefile to detect msysgit and apply specific settings

[Steffen Prohaska]

This commit changes handling of the msysgit specific settings, so
that they can be applied to official git.git.  Some msysgit
settings differ from the standard MinGW settings.  We move them
into an ifndef block that is only evaluated if a file
THIS_IS_MSYSGIT is present in the parent directory, which is the
case for an msysgit working environment.  The tag file is unlikely
to be present accidentally.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 Makefile |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index f9657ce..7803ae7 100644
--- a/Makefile
+++ b/Makefile
@@ -823,7 +823,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	pathsep = ;
 	NO_PREAD = YesPlease
 	NO_OPENSSL = YesPlease
-	NO_CURL = YesPlease
 	NO_SYMLINK_HEAD = YesPlease
 	NO_IPV6 = YesPlease
 	NO_SETENV = YesPlease
@@ -831,7 +830,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_STRCASESTR = YesPlease
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
-	NO_PTHREADS = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	OLD_ICONV = YesPlease
 	NO_C99_FORMAT = YesPlease
@@ -853,6 +851,18 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/regex/regex.o compat/winansi.o
 	EXTLIBS += -lws2_32
 	X = .exe
+ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
+	htmldir=doc/git/html/
+	prefix =
+	INSTALL = /bin/install
+	EXTLIBS += /mingw/lib/libz.a
+	NO_R_TO_GCC_LINKER = YesPlease
+	INTERNAL_QSORT = YesPlease
+	THREADED_DELTA_SEARCH = YesPlease
+else
+	NO_CURL = YesPlease
+	NO_PTHREADS = YesPlease
+endif
 endif
 ifneq (,$(findstring arm,$(uname_M)))
 	ARM_SHA1 = YesPlease
-- 
1.6.3.1.54.g99dd

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> @@ -2541,7 +2543,7 @@ struct malloc_params {
>  static struct malloc_params mparams;
>  
>  /* Ensure mparams initialized */
> -#define ensure_initialization() (mparams.magic != 0 || init_mparams())
> +#define ensure_initialization() if (mparams.magic == 0) init_mparams()
>  
>  #if !ONLY_MSPACES

The code after the patch looks more fragile than the original.  I know
there currently is no code like:

	if (foo())
        	ensure_initialization();
	else
        	warn("oops");

but this change still feels wrong.

What issue is this patch trying to work around?  Returned value not being
used?

> diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> index a381a7d..60a4093 100644
> --- a/compat/nedmalloc/nedmalloc.c
> +++ b/compat/nedmalloc/nedmalloc.c
> @@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
>  /*#define FULLSANITYCHECKS*/
>  
>  #include "nedmalloc.h"
> -#if defined(WIN32) && !defined(__MINGW32__)
> +#if defined(WIN32)
>   #include <malloc.h>
>  #endif

Can somebody enlighten me what this hunk is about, and how it helps GCC
4.4?

There are many "#if[n]def __MINGW32__" remaining in the codebase both
inside and outside compat/ area, so it is not that that symbol is somehow
special.  I cannot even tell which one of the following is closer to the
reason behind this change:

 (1) "Because tacking '&& !defined(__MINGW32__)' after defined(WIN32) is
     unnecessary for such and such reasons, it is removed"; or

 (2) "Because tacking '&& !defined(__MINGW32__)' after defined(WIN32) is
     harmful for such and such reasons, it is removed".

Puzzled.

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Johannes Schindelin]

Hi,

On Mon, 1 Jun 2009, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
> 
> > @@ -2541,7 +2543,7 @@ struct malloc_params {
> >  static struct malloc_params mparams;
> >  
> >  /* Ensure mparams initialized */
> > -#define ensure_initialization() (mparams.magic != 0 || init_mparams())
> > +#define ensure_initialization() if (mparams.magic == 0) init_mparams()
> >  
> >  #if !ONLY_MSPACES
> 
> The code after the patch looks more fragile than the original.  I know
> there currently is no code like:
> 
> 	if (foo())
>         	ensure_initialization();
> 	else
>         	warn("oops");
> 
> but this change still feels wrong.

I know, but the whole use of ensure_initialization() feels wrong to me, 
and I did _not_ want to change the code too much, lest we end up 
maintaining a proper fork as has happened with libxdiff.

> What issue is this patch trying to work around?  Returned value not 
> being used?

I forgot what the GCC warning looked like, and have only text-mode access 
to the web right now, so I cannot access any Windows machine and try 
again.

It was a warning, though, that much I remember.

> > diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> > index a381a7d..60a4093 100644
> > --- a/compat/nedmalloc/nedmalloc.c
> > +++ b/compat/nedmalloc/nedmalloc.c
> > @@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
> >  /*#define FULLSANITYCHECKS*/
> >  
> >  #include "nedmalloc.h"
> > -#if defined(WIN32) && !defined(__MINGW32__)
> > +#if defined(WIN32)
> >   #include <malloc.h>
> >  #endif
> 
> Can somebody enlighten me what this hunk is about, and how it helps GCC
> 4.4?

It helps in that malloc.h is included even if we happen to compile the 
stuff as a MinGW program.  Otherwise necessary function declarations are 
missing.

> There are many "#if[n]def __MINGW32__" remaining in the codebase both
> inside and outside compat/ area, so it is not that that symbol is somehow
> special.  I cannot even tell which one of the following is closer to the
> reason behind this change:
> 
>  (1) "Because tacking '&& !defined(__MINGW32__)' after defined(WIN32) is
>      unnecessary for such and such reasons, it is removed"; or
> 
>  (2) "Because tacking '&& !defined(__MINGW32__)' after defined(WIN32) is
>      harmful for such and such reasons, it is removed".

The latter.

> Puzzled.

Hopefully less so, now.

Ciao,
Dscho

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
>> > index a381a7d..60a4093 100644
>> > --- a/compat/nedmalloc/nedmalloc.c
>> > +++ b/compat/nedmalloc/nedmalloc.c
>> > @@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
>> >  /*#define FULLSANITYCHECKS*/
>> >  
>> >  #include "nedmalloc.h"
>> > -#if defined(WIN32) && !defined(__MINGW32__)
>> > +#if defined(WIN32)
>> >   #include <malloc.h>
>> >  #endif
>> 
>> Can somebody enlighten me what this hunk is about, and how it helps GCC
>> 4.4?
>
> It helps in that malloc.h is included even if we happen to compile the 
> stuff as a MinGW program.  Otherwise necessary function declarations are 
> missing.
> ...
> Hopefully less so, now.

I wish enough information were in the commit log message from the
beginning.

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[GeunSik Lim]

On Tue, Jun 2, 2009 at 1:33 AM, Junio C Hamano <gitster@pobox.com> wrote:

>>> Can somebody enlighten me what this hunk is about, and how it helps GCC
>>> 4.4?
>>
Um... It's strange.
I can not connect to  http://gcc.gnu.org website
to get GCC 4.4 manual(pdf format) currently.
ping is normal just.

[invain@fedora9 invain]$ ping gcc.gnu.org
PING gcc.gnu.org (209.132.176.174) 56(84) bytes of data.
64 bytes from sourceware.org (209.132.176.174): icmp_seq=1 ttl=40 time=194 ms
64 bytes from sourceware.org (209.132.176.174): icmp_seq=2 ttl=40 time=199 ms
^X64 bytes from sourceware.org (209.132.176.174): icmp_seq=3 ttl=40 time=193 ms
64 bytes from sourceware.org (209.132.176.174): icmp_seq=4 ttl=40 time=203 ms
^C
--- gcc.gnu.org ping statistics ---
4 packets transmitted, 4 received, 0% packet loss, time 3326ms
rtt min/avg/max/mdev = 193.762/198.126/203.978/4.135 ms
[invain@fedora9 invain]$
[invain@fedora9 invain]$ nmap  gcc.gnu.org
[invain@fedora9 invain]$ nmap -p 80  gcc.gnu.org

Starting Nmap 4.53 ( http://insecure.org ) at 2009-06-02 08:37 KST
Note: Host seems down. If it is really up, but blocking our ping probes, try -PN
Nmap done: 1 IP address (0 hosts up) scanned in 2.023 seconds
[invain@fedora9 invain]$



-- 
Regards,
GeunSik Lim ( Samsung Electronics )
Blog : http://blog.naver.com/invain/
e-Mail: geunsik.lim@samsung.com
           leemgs@gmail.com , leemgs1@gmail.com

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Johannes Schindelin]

Hi,

On Mon, 1 Jun 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> > diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
> >> > index a381a7d..60a4093 100644
> >> > --- a/compat/nedmalloc/nedmalloc.c
> >> > +++ b/compat/nedmalloc/nedmalloc.c
> >> > @@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
> >> >  /*#define FULLSANITYCHECKS*/
> >> >  
> >> >  #include "nedmalloc.h"
> >> > -#if defined(WIN32) && !defined(__MINGW32__)
> >> > +#if defined(WIN32)
> >> >   #include <malloc.h>
> >> >  #endif
> >> 
> >> Can somebody enlighten me what this hunk is about, and how it helps GCC
> >> 4.4?
> >
> > It helps in that malloc.h is included even if we happen to compile the 
> > stuff as a MinGW program.  Otherwise necessary function declarations are 
> > missing.
> > ...
> > Hopefully less so, now.
> 
> I wish enough information were in the commit log message from the
> beginning.

Okay, how about this in the commit body in addition?

-- snip --
Nedmalloc's source code has a cute #define construct to avoid inserting
an if() statement, because that might interact badly with enclosing if()
statements.  However, there is no danger of that now, but GCC > 4
complains "warning: value computed is not used".

Also, with the version of MinGW's headers in msysGit, we need to include 
malloc.h lest the compiler complain about an "incompatible implicit 
declaration of built-in function 'alloca'".
-- snap --

Hmm?
Dscho

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Okay, how about this in the commit body in addition?
>
> -- snip --
> Nedmalloc's source code has a cute #define construct to avoid inserting
> an if() statement, because that might interact badly with enclosing if()
> statements.  However, there is no danger of that now, but GCC > 4
> complains "warning: value computed is not used".
>
> Also, with the version of MinGW's headers in msysGit, we need to include 
> malloc.h lest the compiler complain about an "incompatible implicit 
> declaration of built-in function 'alloca'".
> -- snap --
>
> Hmm?

As to the first one, I think your problem description is clear (the
problem is clear without being explained to begin with) but "there is no
danger of that now" is somewhere between a lame excuse for not doing it
the right way and a bug waiting to happen.  If "value computed is not
used" is the issue, why doesn't this work?

    #define ensure_initialization() (void)(mparams.magic != 0 || init_mparams())

or at least wrap your fragile "if" in a bog-standard do {} while(0), like...

    #define ensure_initialization() \
    	do { if (mparams.magic == 0) init_mparams(); } while (0)

Then you do not have to make excuses, and instead can just say

	Squelch GCC > 4's "value computed not used" warnings from
	ensure_initialization() macro definition.

As to the second one, I think you lost "even if we happen to" part (that
logically corresponds to "... that is why we do not want to limit the
inclusion to !__MINGW32__ case") from the description, making it less
readable...

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Johannes Schindelin]

Hi,

On Tue, 2 Jun 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Okay, how about this in the commit body in addition?
> >
> > -- snip --
> > Nedmalloc's source code has a cute #define construct to avoid inserting
> > an if() statement, because that might interact badly with enclosing if()
> > statements.  However, there is no danger of that now, but GCC > 4
> > complains "warning: value computed is not used".
> >
> > Also, with the version of MinGW's headers in msysGit, we need to include 
> > malloc.h lest the compiler complain about an "incompatible implicit 
> > declaration of built-in function 'alloca'".
> > -- snap --
> >
> > Hmm?
> 
> As to the first one, I think your problem description is clear (the
> problem is clear without being explained to begin with) but "there is no
> danger of that now" is somewhere between a lame excuse for not doing it
> the right way and a bug waiting to happen.  If "value computed is not
> used" is the issue, why doesn't this work?
> 
>     #define ensure_initialization() (void)(mparams.magic != 0 || init_mparams())

That works.

> As to the second one, I think you lost "even if we happen to" part (that
> logically corresponds to "... that is why we do not want to limit the
> inclusion to !__MINGW32__ case") from the description, making it less
> readable...

You mean "even if we happen to be on Windows"?  Fine with me.  Want me to 
resend a fixed patch?

Ciao,
Dscho

Re: [PATCH 10/11] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> As to the second one, I think you lost "even if we happen to" part (that
>> logically corresponds to "... that is why we do not want to limit the
>> inclusion to !__MINGW32__ case") from the description, making it less
>> readable...
>
> You mean "even if we happen to be on Windows"?

I meant this part from your earlier message.

> It helps in that malloc.h is included even if we happen to compile the 
> stuff as a MinGW program.  Otherwise necessary function declarations are 
> missing.

compared with the newer "how about" version, which is

>> > Also, with the version of MinGW's headers in msysGit, we need to include 
>> > malloc.h lest the compiler complain about an "incompatible implicit 
>> > declaration of built-in function 'alloca'".

I thought the former explains the change in question

>> > -#if defined(WIN32) && !defined(__MINGW32__)
>> > +#if defined(WIN32)

much more clearly: "If you are compiling for Windows, regardless of
MINGW32, you would want this section to apply".

> ...  Want me to resend a fixed patch?

Surely.  I think I could come up with something based on the discussion
here, but I'd rather not.  A patch signed off by either you or Steffen, or
both is very much appreciated.

[PATCH 10/11 v2] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Johannes Schindelin]

Nedmalloc's source code has a cute #define construct to avoid inserting
an if() statement, because that might interact badly with enclosing if()
statements.  However, GCC > 4 complains with a "warning: value computed
is not used".  So we cast the result to "void".

GCC also does not understand the Visual C++ specific pragmas, so we need
to disable them for MinGW.

We need to include malloc.h on Windows even if we happen to compile the
stuff as a MinGW program.  Otherwise the function declaration of alloca()
is missing.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Forgot to send.

 compat/nedmalloc/malloc.c.h  |    4 +++-
 compat/nedmalloc/nedmalloc.c |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
index bb0f482..b5b1495 100644
--- a/compat/nedmalloc/malloc.c.h
+++ b/compat/nedmalloc/malloc.c.h
@@ -1270,7 +1270,9 @@ int mspace_mallopt(int, int);
 /*------------------------------ internal #includes ---------------------- */
 
 #ifdef WIN32
+#ifndef __GNUC__
 #pragma warning( disable : 4146 ) /* no "unsigned" warnings */
+#endif
 #endif /* WIN32 */
 
 #include <stdio.h>       /* for printing in malloc_stats */
@@ -2541,7 +2543,7 @@ struct malloc_params {
 static struct malloc_params mparams;
 
 /* Ensure mparams initialized */
-#define ensure_initialization() (mparams.magic != 0 || init_mparams())
+#define ensure_initialization() ((void)(mparams.magic == 0 || init_mparams()))
 
 #if !ONLY_MSPACES
 
diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 41a3234..d9a17a8 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -34,7 +34,7 @@ DEALINGS IN THE SOFTWARE.
 /*#define FULLSANITYCHECKS*/
 
 #include "nedmalloc.h"
-#if defined(WIN32) && !defined(__MINGW32__)
+#if defined(WIN32)
  #include <malloc.h>
 #endif
 #define MSPACES 1
-- 
1.6.3.284.g6fecc

Re: [PATCH 10/11 v2] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Nedmalloc's source code has a cute #define construct to avoid inserting
> an if() statement, because that might interact badly with enclosing if()
> statements.  However, GCC > 4 complains with a "warning: value computed
> is not used".  So we cast the result to "void".
>
> 	Forgot to send.

Thanks.  The description on the pragma is a good thing to add, which you
did.

Is Visual C++ happy with casting discarded result to (void)?  I'd find it
broken if it isn't, but it would not hurt to ask just to make sure.

Steffen, can we move the series, with this patch replaced, to 'next' now?
I think it looks good (thanks!) but again it would not hurt to ask just to
make sure ;-).

>  compat/nedmalloc/malloc.c.h  |    4 +++-
>  compat/nedmalloc/nedmalloc.c |    2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
> index bb0f482..b5b1495 100644
> --- a/compat/nedmalloc/malloc.c.h
> +++ b/compat/nedmalloc/malloc.c.h
> @@ -1270,7 +1270,9 @@ int mspace_mallopt(int, int);
>  /*------------------------------ internal #includes ---------------------- */
>  
>  #ifdef WIN32
> +#ifndef __GNUC__
>  #pragma warning( disable : 4146 ) /* no "unsigned" warnings */
> +#endif
>  #endif /* WIN32 */
>  
>  #include <stdio.h>       /* for printing in malloc_stats */
> @@ -2541,7 +2543,7 @@ struct malloc_params {
>  static struct malloc_params mparams;
>  
>  /* Ensure mparams initialized */
> -#define ensure_initialization() (mparams.magic != 0 || init_mparams())
> +#define ensure_initialization() ((void)(mparams.magic == 0 || init_mparams()))
>  
>  #if !ONLY_MSPACES
>  

Re: [PATCH 10/11 v2] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Johannes Sixt]

On Montag, 8. Juni 2009, Junio C Hamano wrote:
> Is Visual C++ happy with casting discarded result to (void)?  I'd find it
> broken if it isn't, but it would not hurt to ask just to make sure.

It is OK. Visual C++ is not *that* broken ;)

-- Hannes

Re: [PATCH 10/11 v2] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Steffen Prohaska]

On Jun 8, 2009, at 6:50 PM, Junio C Hamano wrote:

> Steffen, can we move the series, with this patch replaced, to 'next'  
> now?
> I think it looks good (thanks!) but again it would not hurt to ask  
> just to
> make sure ;-).

I agree, it looks good.  Thanks.

	Steffen

Re: [PATCH 10/11 v2] Fix warnings in nedmalloc when compiling with GCC 4.4.0

[Johannes Sixt]

Steffen Prohaska schrieb:
> 
> On Jun 8, 2009, at 6:50 PM, Junio C Hamano wrote:
> 
>> Steffen, can we move the series, with this patch replaced, to 'next' now?
>> I think it looks good (thanks!) but again it would not hurt to ask
>> just to
>> make sure ;-).
> 
> I agree, it looks good.  Thanks.

Please don't advance this series to master just yet: I see severe breakage
with the nedmalloc patch on one of my systems, but I ran out of time to
investigate further. (And I'm afraid I can continue only next Monday.)

-- Hannes

[PATCH] Fix typo in nedmalloc warning fix

[Johannes Sixt]

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  Johannes Sixt schrieb:
  > Please don't advance this series to master just yet: I see severe breakage
  > with the nedmalloc patch on one of my systems, but I ran out of time to
  > investigate further. (And I'm afraid I can continue only next Monday.)

  So, I have the failure on my other system as well, and this is the fix ;)

  -- Hannes

  compat/nedmalloc/malloc.c.h |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
index b5b1495..74c42e3 100644
--- a/compat/nedmalloc/malloc.c.h
+++ b/compat/nedmalloc/malloc.c.h
@@ -2543,7 +2543,7 @@ struct malloc_params {
  static struct malloc_params mparams;

  /* Ensure mparams initialized */
-#define ensure_initialization() ((void)(mparams.magic == 0 || init_mparams()))
+#define ensure_initialization() ((void)(mparams.magic != 0 || init_mparams()))

  #if !ONLY_MSPACES

-- 
1.6.3.2.1225.g177fa.dirty

[PATCH] compat/ has subdirectories: do not omit them in 'make clean'

[Johannes Sixt]

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  Johannes Sixt schrieb:
  >  So, I have the failure on my other system as well, and this is the fix ;)

  And this was quite helpful to find the breakage.

  -- Hannes

  Makefile |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index a38dcdd..7002688 100644
--- a/Makefile
+++ b/Makefile
@@ -1691,7 +1691,7 @@ distclean: clean
  	$(RM) configure

  clean:
-	$(RM) *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o xdiff/*.o \
+	$(RM) *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
  		$(LIB_FILE) $(XDIFF_LIB)
  	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
  	$(RM) $(TEST_PROGRAMS)
-- 
1.6.3.2.1225.g177fa.dirty

Re: [PATCH] Fix typo in nedmalloc warning fix

[Johannes Schindelin]

Hi,

On Thu, 11 Jun 2009, Johannes Sixt wrote:

> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Johannes Sixt schrieb:
>  > Please don't advance this series to master just yet: I see severe breakage
>  > with the nedmalloc patch on one of my systems, but I ran out of time to
>  > investigate further. (And I'm afraid I can continue only next Monday.)
> 
>  So, I have the failure on my other system as well, and this is the fix ;)
> 
>  -- Hannes
> 
>  compat/nedmalloc/malloc.c.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
> index b5b1495..74c42e3 100644
> --- a/compat/nedmalloc/malloc.c.h
> +++ b/compat/nedmalloc/malloc.c.h
> @@ -2543,7 +2543,7 @@ struct malloc_params {
>  static struct malloc_params mparams;
> 
>  /* Ensure mparams initialized */
> -#define ensure_initialization() ((void)(mparams.magic == 0 ||
> init_mparams()))
> +#define ensure_initialization() ((void)(mparams.magic != 0 ||
> init_mparams()))

Aargh!

Thanks for catching this stupid bug,
Dscho

Re: [PATCH 05/11] winansi: fix compile warnings

[Johannes Sixt]

Steffen Prohaska schrieb:
> diff --git a/compat/win32.h b/compat/win32.h
> index c26384e..d531130 100644
> --- a/compat/win32.h
> +++ b/compat/win32.h
> @@ -1,5 +1,6 @@
>  /* common Win32 functions for MinGW and Cygwin */
>  #include <windows.h>
> +#include <conio.h>
>  
>  static inline int file_attr_to_st_mode (DWORD attr)
>  {

I assume this is to remove the warning about missing declaration of 
_getch(). Can we have this patch instead? I don't have conio.h in my 
oldish MinGW environment.

diff --git a/compat/mingw.c b/compat/mingw.c
index 52961ee..53053ad 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1211,6 +1211,9 @@ int link(const char *oldpath, const char *newpath)
  	return 0;
  }

+/* from conio.h */
+int _getch(void);  // FIXME: really look this up in conio.h!!!!
+
  char *getpass(const char *prompt)
  {
  	struct strbuf buf = STRBUF_INIT;

> diff --git a/compat/winansi.c b/compat/winansi.c
> index 4bee335..9217c24 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -80,7 +80,7 @@ static void set_console_attr(void)
>  static void erase_in_line(void)
>  {
>  	CONSOLE_SCREEN_BUFFER_INFO sbi;
> -	long dummy; /* Needed for Windows 7 (or Vista) regression */
> +	DWORD dummy; /* Needed for Windows 7 (or Vista) regression */
>  
>  	if (!console)
>  		return;

This should really be squashed into 03/11, where the line is introduced.

-- Hannes

Re: [PATCH 05/11] winansi: fix compile warnings

[Johannes Sixt]

On Sonntag, 31. Mai 2009, Johannes Sixt wrote:
> Steffen Prohaska schrieb:
> > diff --git a/compat/win32.h b/compat/win32.h
> > index c26384e..d531130 100644
> > --- a/compat/win32.h
> > +++ b/compat/win32.h
> > @@ -1,5 +1,6 @@
> >  /* common Win32 functions for MinGW and Cygwin */
> >  #include <windows.h>
> > +#include <conio.h>
> >
> >  static inline int file_attr_to_st_mode (DWORD attr)
> >  {
>
> I assume this is to remove the warning about missing declaration of
> _getch(). Can we have this patch instead? I don't have conio.h in my
> oldish MinGW environment.
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 52961ee..53053ad 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1211,6 +1211,9 @@ int link(const char *oldpath, const char *newpath)
>   	return 0;
>   }
>
> +/* from conio.h */
> +int _getch(void);  // FIXME: really look this up in conio.h!!!!
> +
>   char *getpass(const char *prompt)
>   {
>   	struct strbuf buf = STRBUF_INIT;

Ok, forget this patch; I'll upgrade my MinGW instead, and let's assume all 
other mingw.git h4ckrz who aren't using the latest msysgit will upgrade as 
well.

Nevertheless, I think that the #include <conio.h> is in the wrong file: it 
should be included from compat/mingw.c.

-- Hannes

[PATCH 05/11 v2] MinGW: fix warning about implicit declaration of _getch()

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

conio.h provides the declaration.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/mingw.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

This replaces 05/11.  The original commit messages was
misleading.  Apparently, the original 05/11 solved two things.
First, it fixed a warning in winansi.c, which has been squashed
into 03/11.  Second, it fixed a warning about implict decl of
_getch().  Including conio.h in mingw.c is sufficient to fix
this warning.

diff --git a/compat/mingw.c b/compat/mingw.c
index e190fdd..12d0c2f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1,5 +1,6 @@
 #include "../git-compat-util.h"
 #include "win32.h"
+#include <conio.h>
 #include "../strbuf.h"
 
 unsigned int _CRT_fmode = _O_BINARY;
-- 
1.6.3.1.54.g99dd

[PATCH 03/11 v2] Work around a regression in Windows 7, causing erase_in_line() to crash sometimes

[Steffen Prohaska]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

The function FillConsoleOutputCharacterA() was pretty content in XP to take a NULL
pointer if we did not want to store the number of written columns.  In Windows 7,
it crashes, but only when called from within Git Bash, not from within cmd.exe.
Go figure.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 compat/winansi.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

I squashed the second chunk of 05/11, as suggested by Hannes.

diff --git a/compat/winansi.c b/compat/winansi.c
index 44dc293..9217c24 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -80,6 +80,7 @@ static void set_console_attr(void)
 static void erase_in_line(void)
 {
 	CONSOLE_SCREEN_BUFFER_INFO sbi;
+	DWORD dummy; /* Needed for Windows 7 (or Vista) regression */
 
 	if (!console)
 		return;
@@ -87,7 +88,7 @@ static void erase_in_line(void)
 	GetConsoleScreenBufferInfo(console, &sbi);
 	FillConsoleOutputCharacterA(console, ' ',
 		sbi.dwSize.X - sbi.dwCursorPosition.X, sbi.dwCursorPosition,
-		NULL);
+		&dummy);
 }
 
 
-- 
1.6.3.1.54.g99dd

Re: [PATCH 03/11 v2] Work around a regression in Windows 7, causing erase_in_line() to crash sometimes

[Johannes Schindelin]

Hi,

On Mon, 1 Jun 2009, Steffen Prohaska wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The function FillConsoleOutputCharacterA() was pretty content in XP to take a NULL
> pointer if we did not want to store the number of written columns.  In Windows 7,
> it crashes, but only when called from within Git Bash, not from within cmd.exe.
> Go figure.

Did I really have too-long lines in my commit message?

> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Steffen Prohaska <prohaska@zib.de>
> ---
>  compat/winansi.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> I squashed the second chunk of 05/11, as suggested by Hannes.

Makes sense, thank you!
Dscho

Re: [PATCH 03/11 v2] Work around a regression in Windows 7, causing erase_in_line() to crash sometimes

[Steffen Prohaska]

On Jun 1, 2009, at 10:25 AM, Johannes Schindelin wrote:

> Hi,
>
> On Mon, 1 Jun 2009, Steffen Prohaska wrote:
>
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> The function FillConsoleOutputCharacterA() was pretty content in XP  
>> to take a NULL
>> pointer if we did not want to store the number of written columns.   
>> In Windows 7,
>> it crashes, but only when called from within Git Bash, not from  
>> within cmd.exe.
>> Go figure.
>
> Did I really have too-long lines in my commit message?


Yes.  See aa3abfc9852a4d4cfaa7f0042102eb56ed2e0daa on 4msysgit's devel.
You are author and committer.

	Steffen

Re: [PATCH 03/11 v2] Work around a regression in Windows 7, causing erase_in_line() to crash sometimes

[Johannes Schindelin]

Hi,

On Mon, 1 Jun 2009, Steffen Prohaska wrote:

> On Jun 1, 2009, at 10:25 AM, Johannes Schindelin wrote:
> 
> >On Mon, 1 Jun 2009, Steffen Prohaska wrote:
> >
> > >From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > >
> > >The function FillConsoleOutputCharacterA() was pretty content in XP to take
> > >a NULL
> > >pointer if we did not want to store the number of written columns.  In
> > >Windows 7,
> > >it crashes, but only when called from within Git Bash, not from within
> > >cmd.exe.
> > >Go figure.
> >
> >Did I really have too-long lines in my commit message?
> 
> 
> Yes.  See aa3abfc9852a4d4cfaa7f0042102eb56ed2e0daa on 4msysgit's devel.
> You are author and committer.

My bad, then!

Ciao,
Dscho

Re: [PATCH 04/11] test-chmtime: work around Windows limitation

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Windows has problems changing the mtime when the file is write protected,
> even by the owner of said file.
>
> Add a Windows-only workaround to change the mode if necessary before
> trying to change the mtime.

I'll queue this as-is for now, but shouldn't the code remember the
original mode and change it back?

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Steffen Prohaska <prohaska@zib.de>
> ---
>  test-chmtime.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/test-chmtime.c b/test-chmtime.c
> index d5358cb..fe476cb 100644
> --- a/test-chmtime.c
> +++ b/test-chmtime.c
> @@ -87,6 +87,15 @@ int main(int argc, const char *argv[])
>  			return -1;
>  		}
>  
> +#ifdef WIN32
> +		if (!(sb.st_mode & S_IWUSR) &&
> +				chmod(argv[i], sb.st_mode | S_IWUSR)) {
> +			fprintf(stderr, "Could not make user-writable %s: %s",
> +				argv[i], strerror(errno));
> +			return -1;
> +		}
> +#endif
> +
>  		utb.actime = sb.st_atime;
>  		utb.modtime = set_eq ? set_time : sb.st_mtime + set_time;
>  
> -- 
> 1.6.3.1.54.g99dd

Re: [PATCH 04/11] test-chmtime: work around Windows limitation

[Johannes Schindelin]

Hi,

On Mon, 1 Jun 2009, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
> 
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > Windows has problems changing the mtime when the file is write protected,
> > even by the owner of said file.
> >
> > Add a Windows-only workaround to change the mode if necessary before
> > trying to change the mtime.
> 
> I'll queue this as-is for now, but shouldn't the code remember the
> original mode and change it back?

Hmm, good point.  As I mentioned in another mail, though, I cannot access 
any Windows machine to fix this issue now, sorry.

Not changing the mode back _might_ paper over another issue: if the files 
are deleted later, that mode change might mean that the deletion now 
succeeds, and fixing test-chmtime may make it fail again.  Not sure, 
though.

Ciao,
Dscho

Re: [PATCH 00/11] Various msysgit patches

[Johannes Sixt]

On Sonntag, 31. Mai 2009, Steffen Prohaska wrote:
> Hello,
> Here are more patches that are in msysgit but not yet in official
> git.git.  The patches have been discussed in the following two
> threads:
>
>     http://thread.gmane.org/gmane.comp.version-control.msysgit/5373
>     http://thread.gmane.org/gmane.comp.version-control.msysgit/4876

Thanks a lot, your work is very much appreciated. Let me add that these 
patches have been in use for weeks, some even for months now.

Please consider to resend updated 03/11 and 05/11 as I have suggested 
elsewhere in the thread.

-- Hannes