[RFC PATCH 0/6] win32-dirent
Erik Faye-Lund
by commit 3ba7a06 ("A loose object is not corrupt if it cannot be read due
to EMFILE").
It unifies the dirent-emulation in compat/mingw.[ch] and compat/msvc.c,
giving us a custom implementation of opendir, readdir and closedir that
does not incorrectly set errno to 0.
Erik Faye-Lund (6):
msvc: opendir: use xmalloc
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 | 105 ++++++++++++++++++++++++++++++++
compat/win32/dirent.h | 24 +++++++
7 files changed, 134 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.ge4bf7
Erik Faye-Lund
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 <kusm...@gmail.com>
---
compat/msvc.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/compat/msvc.c b/compat/msvc.c
index 205304e..38f2d92 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -13,17 +13,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.ge4bf7
Erik Faye-Lund
dd_name by over-allocating. This is not currently done in our
implementation of opendir. Fix this.
Signed-off-by: Erik Faye-Lund <kusm...@gmail.com>
---
compat/msvc.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/compat/msvc.c b/compat/msvc.c
index db3df51..205304e 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -5,12 +5,11 @@
DIR *opendir(const char *name)
{
- int len;
+ int len = strlen(p->dd_name);
DIR *p;
- p = xmalloc(sizeof(DIR));
- memset(p, 0, sizeof(DIR));
- strncpy(p->dd_name, name, PATH_MAX);
- len = strlen(p->dd_name);
+ p = xmalloc(sizeof(DIR) + len + 2);
+ 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.ge4bf7
Erik Faye-Lund
when we should.
Signed-off-by: Erik Faye-Lund <kusm...@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 38f2d92..8417fd3 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -5,8 +5,29 @@
DIR *opendir(const char *name)
{
- int len = strlen(p->dd_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 = xmalloc(sizeof(DIR) + len + 2);
memset(p, 0, sizeof(DIR) + len + 2);
strcpy(p->dd_name, name);
@@ -18,6 +39,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.ge4bf7
Erik Faye-Lund
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/compat/msvc.c b/compat/msvc.c
index ac04a4c..db3df51 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -7,16 +7,13 @@ DIR *opendir(const char *name)
{
int len;
DIR *p;
- p = (DIR*)malloc(sizeof(DIR));
+ p = xmalloc(sizeof(DIR));
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.ge4bf7
Erik Faye-Lund
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/compat/msvc.c b/compat/msvc.c
index 8417fd3..29fd877 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.ge4bf7
Jonathan Nieder
> --- a/compat/msvc.c
> +++ b/compat/msvc.c
> @@ -7,16 +7,13 @@ DIR *opendir(const char *name)
> {
> int len;
> DIR *p;
> - p = (DIR*)malloc(sizeof(DIR));
> + p = xmalloc(sizeof(DIR));
> 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;
A behavior change but maybe a good one. For example, the
prune_packed_objects() loop currently skips object dirs it can't open,
even if that is due to memory exhaustion, but this changes it to error
out.
What is the motivation?
Jonathan Nieder
> --- a/compat/msvc.c
> +++ b/compat/msvc.c
> @@ -5,12 +5,11 @@
>
> DIR *opendir(const char *name)
> {
> - int len;
> + int len = strlen(p->dd_name);
> DIR *p;
Does this work?
Erik Faye-Lund
The motivation is just to avoid having to deal with the error, like we
do other places. It's not a big deal though. I could also set errno to
ENOMEM and return NULL if that's preferable. I just don't see how it
is.
I also slightly dislike setting an error not listed in POSIX'
documentation of opendir, even though it's probably allowed.
Erik Faye-Lund
Whoops. It does work for me, but it shouldn't. I guess malloc is
over-allocating, and git is only using short paths.
Thanks for catching it.
Jonathan Nieder
> --- 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--;
I assume len can't be 0?
Jonathan Nieder
> 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.
Nice! Thanks.
> Makefile | 7 ++-
> compat/mingw.c | 60 ------------------
> compat/mingw.h | 29 ---------
> compat/msvc.c | 49 ---------------
> compat/vcbuild/include/dirent.h | 128 ---------------------------------------
> compat/win32/dirent.c | 105 ++++++++++++++++++++++++++++++++
> compat/win32/dirent.h | 24 +++++++
> 7 files changed, 134 insertions(+), 268 deletions(-)
> delete mode 100644 compat/vcbuild/include/dirent.h
> create mode 100644 compat/win32/dirent.c
> create mode 100644 compat/win32/dirent.h
Does this diff get smaller with -M -C -C -C?
Erik Faye-Lund
Nope, in that case GetFileAttributesA would have errore out.
Erik Faye-Lund
Nope. I guess the reason is that I've adjusted the code a bit while
moving it, because I didn't need to cast back and forth between HANDLE
anymore.
Erik Faye-Lund
Of course, I don't have to set errno; malloc would already have done
that. All I need to do is move the check for NULL a bit earlier, so it
won't segfault on ENOMEM. I'll change it for the next round.
Jonathan Nieder
> The motivation is just to avoid having to deal with the error, like we
> do other places. It's not a big deal though. I could also set errno to
> ENOMEM and return NULL if that's preferable. I just don't see how it
> is.
I don't disagree; just fishing for a commit message. :)
> I also slightly dislike setting an error not listed in POSIX'
> documentation of opendir, even though it's probably allowed.
For future reference, here's what POSIX has to say.
Implementations shall not generate a different error
number from one required by this volume of
POSIX.1-2008 for an error condition described in this
volume of POSIX.1-2008, but may generate additional
errors unless explicitly disallowed for a particular
function.
So ENOMEM would have been allowed from that front.
Erik Faye-Lund
> Erik Faye-Lund wrote:
>
>> The motivation is just to avoid having to deal with the error, like we
>> do other places. It's not a big deal though. I could also set errno to
>> ENOMEM and return NULL if that's preferable. I just don't see how it
>> is.
>
> I don't disagree; just fishing for a commit message. :)
>
Yes, but after stopping to think about it a tad more, I tend to like
it working more similar to existing POSIX implementations.
I am adding a commit message this time, though! ;)
>> I also slightly dislike setting an error not listed in POSIX'
>> documentation of opendir, even though it's probably allowed.
>
> For future reference, here's what POSIX has to say.
>
> Implementations shall not generate a different error
> number from one required by this volume of
> POSIX.1-2008 for an error condition described in this
> volume of POSIX.1-2008, but may generate additional
> errors unless explicitly disallowed for a particular
> function.
>
> So ENOMEM would have been allowed from that front.
>
Thanks for the clarification.
Erik Faye-Lund
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
Erik Faye-Lund
the returned pointer. Move the check a bit earlier to avoid segfault.
Signed-off-by: Erik Faye-Lund <kusm...@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
Erik Faye-Lund
dd_name by over-allocating. This is not currently done in our
implementation of opendir. Fix this.
Signed-off-by: Erik Faye-Lund <kusm...@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
Erik Faye-Lund
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 <kusm...@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
Erik Faye-Lund
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
Erik Faye-Lund
when we should.
Signed-off-by: Erik Faye-Lund <kusm...@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
Erik Faye-Lund
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 <kusm...@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
René Scharfe
> Signed-off-by: Erik Faye-Lund <kusm...@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]))
Erik Faye-Lund
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.
René Scharfe
> On Tue, Nov 23, 2010 at 8:31 PM, Ren� Scharfe
Okay; based on the subject of the patch I had assumed that opendir()
wasn't working at all on paths with trailing slashes.
Ren�
Junio C Hamano
> 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.