[PATCH v2 0/6] win32-dirent

[PATCH v2 0/6] win32-dirent

[Erik Faye-Lund]

Here's a re-roll of my win32-dirent branch, this time with some comments
by Jonathan Nieder taken into account.

* 1/6 is back to using malloc (as opposed to xmalloc),
* 2/6 corrected the strlen, and now does strlen of the input-string. It
  seems I messed up when splitting some patches.

The other patches are adjusted accordingly.

Erik Faye-Lund (6):
  msvc: opendir: fix malloc-failure
  msvc: opendir: allocate enough memory
  msvc: opendir: do not start the search
  win32: dirent: handle errors
  msvc: opendir: handle paths ending with a slash
  win32: use our own dirent.h

 Makefile                        |    7 ++-
 compat/mingw.c                  |   60 ------------------
 compat/mingw.h                  |   29 ---------
 compat/msvc.c                   |   29 ---------
 compat/vcbuild/include/dirent.h |  128 ---------------------------------------
 compat/win32/dirent.c           |  108 +++++++++++++++++++++++++++++++++
 compat/win32/dirent.h           |   24 +++++++
 7 files changed, 137 insertions(+), 248 deletions(-)
 delete mode 100644 compat/vcbuild/include/dirent.h
 create mode 100644 compat/win32/dirent.c
 create mode 100644 compat/win32/dirent.h

-- 
1.7.3.2.493.gc8738

[PATCH v2 1/6] msvc: opendir: fix malloc-failure

[Erik Faye-Lund]

Previsouly, the code checked for malloc-failure after it had accessed
the returned pointer. Move the check a bit earlier to avoid segfault.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/msvc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/compat/msvc.c b/compat/msvc.c
index ac04a4c..d6096e4 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -7,16 +7,16 @@ DIR *opendir(const char *name)
 {
 	int len;
 	DIR *p;
-	p = (DIR*)malloc(sizeof(DIR));
+	p = malloc(sizeof(DIR));
+	if (!p)
+		return NULL;
+
 	memset(p, 0, sizeof(DIR));
 	strncpy(p->dd_name, name, PATH_MAX);
 	len = strlen(p->dd_name);
 	p->dd_name[len] = '/';
 	p->dd_name[len+1] = '*';
 
-	if (p == NULL)
-		return NULL;
-
 	p->dd_handle = _findfirst(p->dd_name, &p->dd_dta);
 
 	if (p->dd_handle == -1) {
-- 
1.7.3.2.493.gc8738

[PATCH v2 2/6] msvc: opendir: allocate enough memory

[Erik Faye-Lund]

The defintion of DIR expects the allocating function to extend
dd_name by over-allocating. This is not currently done in our
implementation of opendir. Fix this.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/msvc.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/compat/msvc.c b/compat/msvc.c
index d6096e4..c195365 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -5,15 +5,14 @@
 
 DIR *opendir(const char *name)
 {
-	int len;
+	int len = strlen(name);
 	DIR *p;
-	p = malloc(sizeof(DIR));
+	p = malloc(sizeof(DIR) + len + 2);
 	if (!p)
 		return NULL;
 
-	memset(p, 0, sizeof(DIR));
-	strncpy(p->dd_name, name, PATH_MAX);
-	len = strlen(p->dd_name);
+	memset(p, 0, sizeof(DIR) + len + 2);
+	strcpy(p->dd_name, name);
 	p->dd_name[len] = '/';
 	p->dd_name[len+1] = '*';
 
-- 
1.7.3.2.493.gc8738

[PATCH v2 3/6] msvc: opendir: do not start the search

[Erik Faye-Lund]

compat/mingw.c's readdir expects to be the one that starts the search,
and if it isn't, then the first entry will be missing or incorrect.

Fix this by removing the call to _findfirst, and initializing dd_handle
to INVALID_HANDLE_VALUE.

At the same time, make sure we use FindClose instead of _findclose,
which is symmetric to readdir's FindFirstFile. Take into account that
the find-handle might already be closed by readdir.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/msvc.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/compat/msvc.c b/compat/msvc.c
index c195365..88c6093 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -16,17 +16,13 @@ DIR *opendir(const char *name)
 	p->dd_name[len] = '/';
 	p->dd_name[len+1] = '*';
 
-	p->dd_handle = _findfirst(p->dd_name, &p->dd_dta);
-
-	if (p->dd_handle == -1) {
-		free(p);
-		return NULL;
-	}
+	p->dd_handle = (long)INVALID_HANDLE_VALUE;
 	return p;
 }
 int closedir(DIR *dir)
 {
-	_findclose(dir->dd_handle);
+	if (dir->dd_handle != (long)INVALID_HANDLE_VALUE)
+		FindClose((HANDLE)dir->dd_handle);
 	free(dir);
 	return 0;
 }
-- 
1.7.3.2.493.gc8738

[PATCH v2 4/6] win32: dirent: handle errors

[Erik Faye-Lund]

Previously all error conditions were ignored. Be nice, and set errno
when we should.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/mingw.c |    2 +-
 compat/msvc.c  |   28 +++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index fdbf093..d8fd5d8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1584,7 +1584,7 @@ struct dirent *mingw_readdir(DIR *dir)
 	HANDLE handle;
 	struct mingw_DIR *mdir = (struct mingw_DIR*)dir;
 
-	if (!dir->dd_handle) {
+	if (!dir || !dir->dd_handle) {
 		errno = EBADF; /* No set_errno for mingw */
 		return NULL;
 	}
diff --git a/compat/msvc.c b/compat/msvc.c
index 88c6093..199eb22 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -5,8 +5,29 @@
 
 DIR *opendir(const char *name)
 {
-	int len = strlen(name);
+	DWORD attrs = GetFileAttributes(name);
+	int len;
 	DIR *p;
+
+	/* check for valid path */
+	if (attrs == INVALID_FILE_ATTRIBUTES) {
+		errno = ENOENT;
+		return NULL;
+	}
+
+	/* check if it's a directory */
+	if (!(attrs & FILE_ATTRIBUTE_DIRECTORY)) {
+		errno = ENOTDIR;
+		return NULL;
+	}
+
+	/* check that the pattern won't be too long for FindFirstFileA */
+	len = strlen(name);
+	if (len + 2 >= MAX_PATH) {
+		errno = ENAMETOOLONG;
+		return NULL;
+	}
+
 	p = malloc(sizeof(DIR) + len + 2);
 	if (!p)
 		return NULL;
@@ -21,6 +42,11 @@ DIR *opendir(const char *name)
 }
 int closedir(DIR *dir)
 {
+	if (!dir) {
+		errno = EBADF;
+		return -1;
+	}
+
 	if (dir->dd_handle != (long)INVALID_HANDLE_VALUE)
 		FindClose((HANDLE)dir->dd_handle);
 	free(dir);
-- 
1.7.3.2.493.gc8738

[PATCH v2 5/6] msvc: opendir: handle paths ending with a slash

[Erik Faye-Lund]

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/msvc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/compat/msvc.c b/compat/msvc.c
index 199eb22..fdbfb70 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -23,6 +23,8 @@ DIR *opendir(const char *name)
 
 	/* check that the pattern won't be too long for FindFirstFileA */
 	len = strlen(name);
+	if (is_dir_sep(name[len - 1]))
+		len--;
 	if (len + 2 >= MAX_PATH) {
 		errno = ENAMETOOLONG;
 		return NULL;
-- 
1.7.3.2.493.gc8738

[PATCH v2 6/6] win32: use our own dirent.h

[Erik Faye-Lund]

The mingw-runtime implemenation of opendir, readdir and closedir
sets errno to 0 on success, something that POSIX explicitly
forbids. 3ba7a06 ("A loose object is not corrupt if it cannot be
read due to EMFILE") introduce a dependency on this behaviour,
leading to a broken "git clone" on Windows.

compat/mingw.c contains an implementation of readdir, and
compat/msvc.c contains implementations of opendir and closedir.

Move these to compat/win32/dirent.[ch], and change to our own DIR
structure at the same time.

This provides a generic Win32-implementation of opendir, readdir
and closedir which works on both MinGW and MSVC and does not reset
errno, and as a result git clone is working again on Windows.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 Makefile                        |    7 ++-
 compat/mingw.c                  |   60 ------------------
 compat/mingw.h                  |   29 ---------
 compat/msvc.c                   |   52 ----------------
 compat/vcbuild/include/dirent.h |  128 ---------------------------------------
 compat/win32/dirent.c           |  108 +++++++++++++++++++++++++++++++++
 compat/win32/dirent.h           |   24 +++++++
 7 files changed, 137 insertions(+), 271 deletions(-)
 delete mode 100644 compat/vcbuild/include/dirent.h
 create mode 100644 compat/win32/dirent.c
 create mode 100644 compat/win32/dirent.h

diff --git a/Makefile b/Makefile
index ca1d1fe..765aad4 100644
--- a/Makefile
+++ b/Makefile
@@ -499,6 +499,7 @@ LIB_H += compat/mingw.h
 LIB_H += compat/win32/pthread.h
 LIB_H += compat/win32/syslog.h
 LIB_H += compat/win32/sys/poll.h
+LIB_H += compat/win32/dirent.h
 LIB_H += csum-file.h
 LIB_H += decorate.h
 LIB_H += delta.h
@@ -1086,7 +1087,9 @@ ifeq ($(uname_S),Windows)
 	AR = compat/vcbuild/scripts/lib.pl
 	CFLAGS =
 	BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
-	COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o compat/win32/syslog.o compat/win32/sys/poll.o
+	COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o \
+		compat/win32/pthread.o compat/win32/syslog.o \
+		compat/win32/sys/poll.o compat/win32/dirent.o
 	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
 	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
 	EXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib
@@ -1158,7 +1161,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o \
 		compat/win32/pthread.o compat/win32/syslog.o \
-		compat/win32/sys/poll.o
+		compat/win32/sys/poll.o compat/win32/dirent.o
 	EXTLIBS += -lws2_32
 	PTHREAD_LIBS =
 	X = .exe
diff --git a/compat/mingw.c b/compat/mingw.c
index d8fd5d8..bee6054 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1566,63 +1566,3 @@ pid_t waitpid(pid_t pid, int *status, unsigned options)
 	errno = EINVAL;
 	return -1;
 }
-
-#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 || !dir->dd_handle) {
-		errno = EBADF; /* No set_errno for mingw */
-		return NULL;
-	}
-
-	if (dir->dd_handle == (long)INVALID_HANDLE_VALUE && dir->dd_stat == 0)
-	{
-		DWORD lasterr;
-		handle = FindFirstFileA(dir->dd_name, &buf);
-		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 35d9813..2283071 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -322,35 +322,6 @@ int main(int argc, const char **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
-
 /*
  * Used by Pthread API implementation for Windows
  */
diff --git a/compat/msvc.c b/compat/msvc.c
index fdbfb70..71843d7 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -3,56 +3,4 @@
 #include <conio.h>
 #include "../strbuf.h"
 
-DIR *opendir(const char *name)
-{
-	DWORD attrs = GetFileAttributes(name);
-	int len;
-	DIR *p;
-
-	/* check for valid path */
-	if (attrs == INVALID_FILE_ATTRIBUTES) {
-		errno = ENOENT;
-		return NULL;
-	}
-
-	/* check if it's a directory */
-	if (!(attrs & FILE_ATTRIBUTE_DIRECTORY)) {
-		errno = ENOTDIR;
-		return NULL;
-	}
-
-	/* check that the pattern won't be too long for FindFirstFileA */
-	len = strlen(name);
-	if (is_dir_sep(name[len - 1]))
-		len--;
-	if (len + 2 >= MAX_PATH) {
-		errno = ENAMETOOLONG;
-		return NULL;
-	}
-
-	p = malloc(sizeof(DIR) + len + 2);
-	if (!p)
-		return NULL;
-
-	memset(p, 0, sizeof(DIR) + len + 2);
-	strcpy(p->dd_name, name);
-	p->dd_name[len] = '/';
-	p->dd_name[len+1] = '*';
-
-	p->dd_handle = (long)INVALID_HANDLE_VALUE;
-	return p;
-}
-int closedir(DIR *dir)
-{
-	if (!dir) {
-		errno = EBADF;
-		return -1;
-	}
-
-	if (dir->dd_handle != (long)INVALID_HANDLE_VALUE)
-		FindClose((HANDLE)dir->dd_handle);
-	free(dir);
-	return 0;
-}
-
 #include "mingw.c"
diff --git a/compat/vcbuild/include/dirent.h b/compat/vcbuild/include/dirent.h
deleted file mode 100644
index 440618d..0000000
--- a/compat/vcbuild/include/dirent.h
+++ /dev/null
@@ -1,128 +0,0 @@
-/*
- * DIRENT.H (formerly DIRLIB.H)
- * This file has no copyright assigned and is placed in the Public Domain.
- * This file is a part of the mingw-runtime package.
- *
- * The mingw-runtime package and its code is distributed in the hope that it
- * will be useful but WITHOUT ANY WARRANTY.  ALL WARRANTIES, EXPRESSED OR
- * IMPLIED ARE HEREBY DISCLAIMED.  This includes but is not limited to
- * warranties of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You are free to use this package and its code without limitation.
- */
-#ifndef _DIRENT_H_
-#define _DIRENT_H_
-#include <io.h>
-
-#define PATH_MAX 512
-
-#define __MINGW_NOTHROW
-
-#ifndef RC_INVOKED
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-struct dirent
-{
-	long		d_ino;		/* Always zero. */
-	unsigned short	d_reclen;	/* Always zero. */
-	unsigned short	d_namlen;	/* Length of name in d_name. */
-	char		d_name[FILENAME_MAX]; /* File name. */
-};
-
-/*
- * This is an internal data structure. Good programmers will not use it
- * except as an argument to one of the functions below.
- * dd_stat field is now int (was short in older versions).
- */
-typedef struct
-{
-	/* disk transfer area for this dir */
-	struct _finddata_t	dd_dta;
-
-	/* dirent struct to return from dir (NOTE: this makes this thread
-	 * safe as long as only one thread uses a particular DIR struct at
-	 * a time) */
-	struct dirent		dd_dir;
-
-	/* _findnext handle */
-	long			dd_handle;
-
-	/*
-	 * Status of search:
-	 *   0 = not started yet (next entry to read is first entry)
-	 *  -1 = off the end
-	 *   positive = 0 based index of next entry
-	 */
-	int			dd_stat;
-
-	/* given path for dir with search pattern (struct is extended) */
-	char			dd_name[PATH_MAX+3];
-} DIR;
-
-DIR* __cdecl __MINGW_NOTHROW opendir (const char*);
-struct dirent* __cdecl __MINGW_NOTHROW readdir (DIR*);
-int __cdecl __MINGW_NOTHROW closedir (DIR*);
-void __cdecl __MINGW_NOTHROW rewinddir (DIR*);
-long __cdecl __MINGW_NOTHROW telldir (DIR*);
-void __cdecl __MINGW_NOTHROW seekdir (DIR*, long);
-
-
-/* wide char versions */
-
-struct _wdirent
-{
-	long		d_ino;		/* Always zero. */
-	unsigned short	d_reclen;	/* Always zero. */
-	unsigned short	d_namlen;	/* Length of name in d_name. */
-	wchar_t		d_name[FILENAME_MAX]; /* File name. */
-};
-
-/*
- * This is an internal data structure. Good programmers will not use it
- * except as an argument to one of the functions below.
- */
-typedef struct
-{
-	/* disk transfer area for this dir */
-	//struct _wfinddata_t	dd_dta;
-
-	/* dirent struct to return from dir (NOTE: this makes this thread
-	 * safe as long as only one thread uses a particular DIR struct at
-	 * a time) */
-	struct _wdirent		dd_dir;
-
-	/* _findnext handle */
-	long			dd_handle;
-
-	/*
-	 * Status of search:
-	 *   0 = not started yet (next entry to read is first entry)
-	 *  -1 = off the end
-	 *   positive = 0 based index of next entry
-	 */
-	int			dd_stat;
-
-	/* given path for dir with search pattern (struct is extended) */
-	wchar_t			dd_name[1];
-} _WDIR;
-
-
-
-_WDIR* __cdecl __MINGW_NOTHROW _wopendir (const wchar_t*);
-struct _wdirent*  __cdecl __MINGW_NOTHROW _wreaddir (_WDIR*);
-int __cdecl __MINGW_NOTHROW _wclosedir (_WDIR*);
-void __cdecl __MINGW_NOTHROW _wrewinddir (_WDIR*);
-long __cdecl __MINGW_NOTHROW _wtelldir (_WDIR*);
-void __cdecl __MINGW_NOTHROW _wseekdir (_WDIR*, long);
-
-
-#ifdef	__cplusplus
-}
-#endif
-
-#endif	/* Not RC_INVOKED */
-
-#endif	/* Not _DIRENT_H_ */
diff --git a/compat/win32/dirent.c b/compat/win32/dirent.c
new file mode 100644
index 0000000..7a0debe
--- /dev/null
+++ b/compat/win32/dirent.c
@@ -0,0 +1,108 @@
+#include "../git-compat-util.h"
+#include "dirent.h"
+
+struct DIR {
+	struct dirent dd_dir; /* includes d_type */
+	HANDLE dd_handle;     /* FindFirstFile handle */
+	int dd_stat;          /* 0-based index */
+	char dd_name[1];      /* extend struct */
+};
+
+DIR *opendir(const char *name)
+{
+	DWORD attrs = GetFileAttributesA(name);
+	int len;
+	DIR *p;
+
+	/* check for valid path */
+	if (attrs == INVALID_FILE_ATTRIBUTES) {
+		errno = ENOENT;
+		return NULL;
+	}
+
+	/* check if it's a directory */
+	if (!(attrs & FILE_ATTRIBUTE_DIRECTORY)) {
+		errno = ENOTDIR;
+		return NULL;
+	}
+
+	/* check that the pattern won't be too long for FindFirstFileA */
+	len = strlen(name);
+	if (is_dir_sep(name[len - 1]))
+		len--;
+	if (len + 2 >= MAX_PATH) {
+		errno = ENAMETOOLONG;
+		return NULL;
+	}
+
+	p = malloc(sizeof(DIR) + len + 2);
+	if (!p)
+		return NULL;
+
+	memset(p, 0, sizeof(DIR) + len + 2);
+	strcpy(p->dd_name, name);
+	p->dd_name[len] = '/';
+	p->dd_name[len+1] = '*';
+
+	p->dd_handle = INVALID_HANDLE_VALUE;
+	return p;
+}
+
+struct dirent *readdir(DIR *dir)
+{
+	WIN32_FIND_DATAA buf;
+	HANDLE handle;
+
+	if (!dir || !dir->dd_handle) {
+		errno = EBADF; /* No set_errno for mingw */
+		return NULL;
+	}
+
+	if (dir->dd_handle == INVALID_HANDLE_VALUE && dir->dd_stat == 0) {
+		DWORD lasterr;
+		handle = FindFirstFileA(dir->dd_name, &buf);
+		lasterr = GetLastError();
+		dir->dd_handle = handle;
+		if (handle == INVALID_HANDLE_VALUE && (lasterr != ERROR_NO_MORE_FILES)) {
+			errno = err_win_to_posix(lasterr);
+			return NULL;
+		}
+	} else if (dir->dd_handle == INVALID_HANDLE_VALUE) {
+		return NULL;
+	} else if (!FindNextFileA(dir->dd_handle, &buf)) {
+		DWORD lasterr = GetLastError();
+		FindClose(dir->dd_handle);
+		dir->dd_handle = 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 */
+	dir->dd_dir.d_type = 0;
+	if (buf.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+		dir->dd_dir.d_type |= DT_DIR;
+	else
+		dir->dd_dir.d_type |= DT_REG;
+
+	return &dir->dd_dir;
+}
+
+int closedir(DIR *dir)
+{
+	if (!dir) {
+		errno = EBADF;
+		return -1;
+	}
+
+	if (dir->dd_handle != INVALID_HANDLE_VALUE)
+		FindClose(dir->dd_handle);
+	free(dir);
+	return 0;
+}
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
new file mode 100644
index 0000000..927a25c
--- /dev/null
+++ b/compat/win32/dirent.h
@@ -0,0 +1,24 @@
+#ifndef DIRENT_H
+#define DIRENT_H
+
+typedef struct DIR DIR;
+
+#define DT_UNKNOWN 0
+#define DT_DIR     1
+#define DT_REG     2
+#define DT_LNK     3
+
+struct dirent {
+	long d_ino;                      /* Always zero. */
+	char d_name[FILENAME_MAX];       /* File name. */
+	union {
+		unsigned short d_reclen; /* Always zero. */
+		unsigned char  d_type;   /* Reimplementation adds this */
+	};
+};
+
+DIR *opendir(const char *dirname);
+struct dirent *readdir(DIR *dir);
+int closedir(DIR *dir);
+
+#endif /* DIRENT_H */
-- 
1.7.3.2.493.gc8738

Re: [PATCH v2 5/6] msvc: opendir: handle paths ending with a slash

[René Scharfe]

Am 23.11.2010 19:38, schrieb Erik Faye-Lund:
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>  compat/msvc.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/compat/msvc.c b/compat/msvc.c
> index 199eb22..fdbfb70 100644
> --- a/compat/msvc.c
> +++ b/compat/msvc.c
> @@ -23,6 +23,8 @@ DIR *opendir(const char *name)
>  
>  	/* check that the pattern won't be too long for FindFirstFileA */
>  	len = strlen(name);
> +	if (is_dir_sep(name[len - 1]))

Perhaps extend this thus, and handle multiple slashes?

	while (len > 0 && is_dir_sep(name[len - 1]))

> +		len--;
>  	if (len + 2 >= MAX_PATH) {
>  		errno = ENAMETOOLONG;
>  		return NULL;

Re: [PATCH v2 5/6] msvc: opendir: handle paths ending with a slash

[Erik Faye-Lund]

On Tue, Nov 23, 2010 at 8:31 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 23.11.2010 19:38, schrieb Erik Faye-Lund:
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
>>  compat/msvc.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/compat/msvc.c b/compat/msvc.c
>> index 199eb22..fdbfb70 100644
>> --- a/compat/msvc.c
>> +++ b/compat/msvc.c
>> @@ -23,6 +23,8 @@ DIR *opendir(const char *name)
>>
>>       /* check that the pattern won't be too long for FindFirstFileA */
>>       len = strlen(name);
>> +     if (is_dir_sep(name[len - 1]))
>
> Perhaps extend this thus, and handle multiple slashes?
>
>        while (len > 0 && is_dir_sep(name[len - 1]))
>
>> +             len--;
>>       if (len + 2 >= MAX_PATH) {
>>               errno = ENAMETOOLONG;
>>               return NULL;

Sure, it could be done. But the reason why I didn't do that myself was
that I got a bit worried about this being a slippery slope towards
full path-normalization. FindFirstFileA() does handle non-normalized
paths, as long as they are below MAX_PATH in length.

I just didn't want to add a slash if there was already one there,
which is something different than trying to fix the input-path.

In other words, I'm not entirely sure if fixing up such a case is a
task that belongs to opendir.

Re: [PATCH v2 5/6] msvc: opendir: handle paths ending with a slash

[René Scharfe]

Am 23.11.2010 20:48, schrieb Erik Faye-Lund:
> On Tue, Nov 23, 2010 at 8:31 PM, René Scharfe
> <rene.scharfe@lsrfire.ath.cx> wrote:
>> Am 23.11.2010 19:38, schrieb Erik Faye-Lund:
>>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>>> ---
>>>  compat/msvc.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/compat/msvc.c b/compat/msvc.c
>>> index 199eb22..fdbfb70 100644
>>> --- a/compat/msvc.c
>>> +++ b/compat/msvc.c
>>> @@ -23,6 +23,8 @@ DIR *opendir(const char *name)
>>>
>>>       /* check that the pattern won't be too long for FindFirstFileA */
>>>       len = strlen(name);
>>> +     if (is_dir_sep(name[len - 1]))
>>
>> Perhaps extend this thus, and handle multiple slashes?
>>
>>        while (len > 0 && is_dir_sep(name[len - 1]))
>>
>>> +             len--;
>>>       if (len + 2 >= MAX_PATH) {
>>>               errno = ENAMETOOLONG;
>>>               return NULL;
> 
> Sure, it could be done. But the reason why I didn't do that myself was
> that I got a bit worried about this being a slippery slope towards
> full path-normalization. FindFirstFileA() does handle non-normalized
> paths, as long as they are below MAX_PATH in length.
> 
> I just didn't want to add a slash if there was already one there,
> which is something different than trying to fix the input-path.
> 
> In other words, I'm not entirely sure if fixing up such a case is a
> task that belongs to opendir.

Okay; based on the subject of the patch I had assumed that opendir()
wasn't working at all on paths with trailing slashes.

René

Re: [PATCH v2 2/6] msvc: opendir: allocate enough memory

[Junio C Hamano]

Erik Faye-Lund <kusmabite@gmail.com> writes:

>  DIR *opendir(const char *name)
>  {
> -	int len;
> +	int len = strlen(name);
>  	DIR *p;
> -	p = malloc(sizeof(DIR));
> +	p = malloc(sizeof(DIR) + len + 2);
>  	if (!p)
>  		return NULL;
>  
> -	memset(p, 0, sizeof(DIR));
> -	strncpy(p->dd_name, name, PATH_MAX);
> -	len = strlen(p->dd_name);
> +	memset(p, 0, sizeof(DIR) + len + 2);
> +	strcpy(p->dd_name, name);

I'd queue this anyway, but I think you do not have to memset() that many
bytes.  It should be enough to clear sizeof(DIR) here; you have to assign
NUL p->dd_name[len+2] though.

>  	p->dd_name[len] = '/';
>  	p->dd_name[len+1] = '*';
>  
> -- 
> 1.7.3.2.493.gc8738