Re: [msysGit] Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k

2014-05-20 Thread Erik Faye-Lund 
On Tue, May 20, 2014 at 10:46 AM, Thomas Braun
thomas.br...@byte-physics.de wrote:
 Am 19.05.2014 22:29, schrieb Erik Faye-Lund:
  [...]
 Would we need to wrap both ends, shouldn't wrapping only reading be
 good enough to prevent deadlocking?

 compat/poll/poll.c already contains a function called IsSocketHandle
 that is able to tell if a HANDLE points to a socket or not.

 This very quick attempt did not work out :(

 diff --git a/compat/mingw.c b/compat/mingw.c
 index 0335958..ec1d81f 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)
   return fd;
   }

 +#define is_console_handle(h) (((long) (h)  3) == 3)
 +
 +static int is_socket_handle(HANDLE h)
 +{
 +WSANETWORKEVENTS ev;
 +
 +if (is_console_handle(h))
 +return 0;
 +
 +/*
 + * Under Wine, it seems that getsockopt returns 0 for pipes too.
 + * WSAEnumNetworkEvents instead distinguishes the two correctly.
 + */
 +ev.lNetworkEvents = 0xDEADBEEF;
 +WSAEnumNetworkEvents((SOCKET) h, NULL, ev);
 +return ev.lNetworkEvents != 0xDEADBEEF;
 +}
 +
 +#undef read
 +ssize_t mingw_read(int fd, void *buf, size_t count)
 +{
 +int ret;
 +HANDLE fh = (HANDLE)_get_osfhandle(fd);
 +
 +if (fh == INVALID_HANDLE_VALUE) {
 +errno = EBADF;
 +return -1;
 +}
 +
 +if (!is_socket_handle(fh))
 +return read(fd, buf, count);
 +
 +ret = recv((SOCKET)fh, buf, count, 0);
 +if (ret  0)
 +errno = WSAGetLastError();
 +return ret;
 +}
 +
 +#undef write
 +ssize_t mingw_write(int fd, const void *buf, size_t count)
 +{
 +int ret;
 +HANDLE fh = (HANDLE)_get_osfhandle(fd);
 +
 +if (fh == INVALID_HANDLE_VALUE) {
 +errno = EBADF;
 +return -1;
 +}
 +
 +if (!is_socket_handle(fh))
 +return write(fd, buf, count);
 +
 +return send((SOCKET)fh, buf, count, 0);
 +if (ret  0)
 +errno = WSAGetLastError();
 +return ret;
 +}
 +
 +
   static BOOL WINAPI ctrl_ignore(DWORD type)
   {
   return TRUE;
 diff --git a/compat/mingw.h b/compat/mingw.h
 index 08b83fe..1690098 100644
 --- a/compat/mingw.h
 +++ b/compat/mingw.h
 @@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);
   int mingw_open (const char *filename, int oflags, ...);
   #define open mingw_open

 +ssize_t mingw_read(int fd, void *buf, size_t count);
 +#define read mingw_read
 +
 +ssize_t mingw_write(int fd, const void *buf, size_t count);
 +#define write mingw_write
 +
   int mingw_fgetc(FILE *stream);
   #define fgetc mingw_fgetc

 According to [1] you also have to pass WSA_FLAG_OVERLAPPED to avoid the 
 deadlock.

 With that change I don't get a hang anymore but a read error with
 errno 10054 aka WSAECONNRESET.

 [1]: https://groups.google.com/forum/#!msg/msysgit/at8D7J-h7mw/PM9w-d41cDYJ


Yeah, sorry, I noticed this right after sending and tested with that
as well. My results were the same :/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [msysGit] Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k

2014-05-19 Thread Erik Faye-Lund 
On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Thomas Braun wrote:

 pushing over the dump git protocol with a windows git client.

 I've never heard of the dump git protocol.  Do you mean the git
 protocol that's used with git:// URLs?

 [...]
 Alternative approaches considered but deemed too invasive:
 - Rewrite read/write wrappers in mingw.c in order to distinguish between
   a file descriptor which has a socket behind and a file descriptor
   which has a file behind.

 I assume here too invasive means too much engineering effort?

 It sounds like a clean fix, not too invasive at all.  But I can
 understand wanting a stopgap in the meantime.


Yeah, now that the problem seems to be understood, I don't think that
would be too bad. I recently killed off our previous write()-wrapper
in c9df6f4, but I see no reason why we can't add a new one.

Would we need to wrap both ends, shouldn't wrapping only reading be
good enough to prevent deadlocking?

compat/poll/poll.c already contains a function called IsSocketHandle
that is able to tell if a HANDLE points to a socket or not.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [msysGit] Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k

2014-05-19 Thread Erik Faye-Lund 
On Mon, May 19, 2014 at 10:00 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Thomas Braun wrote:

 pushing over the dump git protocol with a windows git client.

 I've never heard of the dump git protocol.  Do you mean the git
 protocol that's used with git:// URLs?

 [...]
 Alternative approaches considered but deemed too invasive:
 - Rewrite read/write wrappers in mingw.c in order to distinguish between
   a file descriptor which has a socket behind and a file descriptor
   which has a file behind.

 I assume here too invasive means too much engineering effort?

 It sounds like a clean fix, not too invasive at all.  But I can
 understand wanting a stopgap in the meantime.


 Yeah, now that the problem seems to be understood, I don't think that
 would be too bad. I recently killed off our previous write()-wrapper
 in c9df6f4, but I see no reason why we can't add a new one.

 Would we need to wrap both ends, shouldn't wrapping only reading be
 good enough to prevent deadlocking?

 compat/poll/poll.c already contains a function called IsSocketHandle
 that is able to tell if a HANDLE points to a socket or not.

This very quick attempt did not work out :(

diff --git a/compat/mingw.c b/compat/mingw.c
index 0335958..ec1d81f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)
 return fd;
 }

+#define is_console_handle(h) (((long) (h)  3) == 3)
+
+static int is_socket_handle(HANDLE h)
+{
+WSANETWORKEVENTS ev;
+
+if (is_console_handle(h))
+return 0;
+
+/*
+ * Under Wine, it seems that getsockopt returns 0 for pipes too.
+ * WSAEnumNetworkEvents instead distinguishes the two correctly.
+ */
+ev.lNetworkEvents = 0xDEADBEEF;
+WSAEnumNetworkEvents((SOCKET) h, NULL, ev);
+return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
+#undef read
+ssize_t mingw_read(int fd, void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return read(fd, buf, count);
+
+ret = recv((SOCKET)fh, buf, count, 0);
+if (ret  0)
+errno = WSAGetLastError();
+return ret;
+}
+
+#undef write
+ssize_t mingw_write(int fd, const void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return write(fd, buf, count);
+
+return send((SOCKET)fh, buf, count, 0);
+if (ret  0)
+errno = WSAGetLastError();
+return ret;
+}
+
+
 static BOOL WINAPI ctrl_ignore(DWORD type)
 {
 return TRUE;
diff --git a/compat/mingw.h b/compat/mingw.h
index 08b83fe..1690098 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);
 int mingw_open (const char *filename, int oflags, ...);
 #define open mingw_open

+ssize_t mingw_read(int fd, void *buf, size_t count);
+#define read mingw_read
+
+ssize_t mingw_write(int fd, const void *buf, size_t count);
+#define write mingw_write
+
 int mingw_fgetc(FILE *stream);
 #define fgetc mingw_fgetc
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html