threads / patch / 40198

patchMingw: verify both ends of the pipe () call

Subject: [PATCH] Mingw: verify both ends of the pipe () call

## tl;dr

9 messages between Aug 27, 2015 and Aug 28, 2015. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Johannes Schindelin· Aug 27, 2015, 15:55 UTC · lore
From: jfmc <jfmcjf@gmail.com>

The code to open and test the second end of the pipe clearly imitates the code for the first end. A little too closely, though... Let's fix the obvious copy-edit bug.

Signed-off-by: Jose F. Morales <jfmcjf@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/mingw.c +1 −2
diff --git a/compat/mingw.c b/compat/mingw.c
index 496e6f8..f74da23 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -681,7 +681,7 @@ int pipe(int filedes[2])
 		return -1;
 	}
 	filedes[1] = _open_osfhandle((int)h[1], O_NOINHERIT);
-	if (filedes[0] < 0) {
+	if (filedes[1] < 0) {
 		close(filedes[0]);
 		CloseHandle(h[1]);
 		return -1;

--
https://github.com/git/git/pull/168
Johannes Schindelin· Aug 27, 2015, 16:09 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Mingw: verify both ends of the pipe () call

Hi all,
On 2015-08-27 17:55, Johannes Schindelin wrote:
Show 9 quoted lines
> From: jfmc <jfmcjf@gmail.com>
> 
> The code to open and test the second end of the pipe clearly imitates
> the code for the first end. A little too closely, though... Let's fix
> the obvious copy-edit bug.
> 
> Signed-off-by: Jose F. Morales <jfmcjf@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
For what it's worth: this is my first submitGit submission! ;-)

Ciao, Dscho

Jonathan Nieder· Aug 27, 2015, 21:50 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Mingw: verify both ends of the pipe () call

Johannes Schindelin wrote:
> From: jfmc <jfmcjf@gmail.com>

This means the name shown by git shortlog would be jfmc instead of Jose F. Morales. Intended?

Show 9 quoted lines
> The code to open and test the second end of the pipe clearly imitates
> the code for the first end. A little too closely, though... Let's fix
> the obvious copy-edit bug.
>
> Signed-off-by: Jose F. Morales <jfmcjf@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

This is an old one --- more than 5 years old (since v1.7.0-rc0~86^2~4 "Windows: simplify the pipe(2) implementation", 2010-01-15). Thanks for catching it.

Regards, Jonathan

(patch kept unsnipped for reference)
Show 16 quoted lines
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 496e6f8..f74da23 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -681,7 +681,7 @@ int pipe(int filedes[2])
>  		return -1;
>  	}
>  	filedes[1] = _open_osfhandle((int)h[1], O_NOINHERIT);
> -	if (filedes[0] < 0) {
> +	if (filedes[1] < 0) {
>  		close(filedes[0]);
>  		CloseHandle(h[1]);
>  		return -1;
> 
> --
> https://github.com/git/git/pull/168
Johannes Sixt· Aug 28, 2015, 08:07 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Mingw: verify both ends of the pipe () call

Am 27.08.2015 um 23:50 schrieb Jonathan Nieder:
Show 22 quoted lines
> Johannes Schindelin wrote:
>
>> From: jfmc <jfmcjf@gmail.com>
>
> This means the name shown by git shortlog would be jfmc instead of
> Jose F. Morales.  Intended?
>
>> The code to open and test the second end of the pipe clearly imitates
>> the code for the first end. A little too closely, though... Let's fix
>> the obvious copy-edit bug.
>>
>> Signed-off-by: Jose F. Morales <jfmcjf@gmail.com>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>>   compat/mingw.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> This is an old one --- more than 5 years old (since v1.7.0-rc0~86^2~4
> "Windows: simplify the pipe(2) implementation", 2010-01-15).  Thanks
> for catching it.
Ouch! Thanks for cleaning up the mess I left behind.
-- Hannes
Show 20 quoted lines
>
> Regards,
> Jonathan
>
> (patch kept unsnipped for reference)
>
>> diff --git a/compat/mingw.c b/compat/mingw.c
>> index 496e6f8..f74da23 100644
>> --- a/compat/mingw.c
>> +++ b/compat/mingw.c
>> @@ -681,7 +681,7 @@ int pipe(int filedes[2])
>>   		return -1;
>>   	}
>>   	filedes[1] = _open_osfhandle((int)h[1], O_NOINHERIT);
>> -	if (filedes[0] < 0) {
>> +	if (filedes[1] < 0) {
>>   		close(filedes[0]);
>>   		CloseHandle(h[1]);
>>   		return -1;
>>
Johannes Schindelin· Aug 28, 2015, 09:52 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Mingw: verify both ends of the pipe () call

Hi Jonathan,
On 2015-08-27 23:50, Jonathan Nieder wrote:
Show 6 quoted lines
> Johannes Schindelin wrote:
> 
>> From: jfmc <jfmcjf@gmail.com>
> 
> This means the name shown by git shortlog would be jfmc instead of
> Jose F. Morales.  Intended?
Fixed in v2 ;-)

Ciao, Dscho

Johannes Schindelin· Aug 28, 2015, 09:43 UTC · re: Johannes Schindelin · lore

[PATCH v2] Mingw: verify both ends of the pipe () call

From: Jose F. Morales <jfmcjf@gmail.com>

The code to open and test the second end of the pipe clearly imitates the code for the first end. A little too closely, though... Let's fix the obvious copy-edit bug.

Signed-off-by: Jose F. Morales <jfmcjf@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/mingw.c +1 −2
diff --git a/compat/mingw.c b/compat/mingw.c
index 496e6f8..f74da23 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -681,7 +681,7 @@ int pipe(int filedes[2])
 		return -1;
 	}
 	filedes[1] = _open_osfhandle((int)h[1], O_NOINHERIT);
-	if (filedes[0] < 0) {
+	if (filedes[1] < 0) {
 		close(filedes[0]);
 		CloseHandle(h[1]);
 		return -1;

--
https://github.com/git/git/pull/168
Junio C Hamano· Aug 28, 2015, 18:14 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2] Mingw: verify both ends of the pipe () call

Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 28 quoted lines
> From: Jose F. Morales <jfmcjf@gmail.com>
>
> The code to open and test the second end of the pipe clearly imitates
> the code for the first end. A little too closely, though... Let's fix
> the obvious copy-edit bug.
>
> Signed-off-by: Jose F. Morales <jfmcjf@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 496e6f8..f74da23 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -681,7 +681,7 @@ int pipe(int filedes[2])
>  		return -1;
>  	}
>  	filedes[1] = _open_osfhandle((int)h[1], O_NOINHERIT);
> -	if (filedes[0] < 0) {
> +	if (filedes[1] < 0) {
>  		close(filedes[0]);
>  		CloseHandle(h[1]);
>  		return -1;
>
> --
> https://github.com/git/git/pull/168

Thanks all. Will queue directly on 'maint' with reviewed-by's and acked-by's.

Johannes Schindelin· Aug 28, 2015, 09:39 UTC · lore

Re: [PATCH] Mingw: verify both ends of the pipe () call

Hi Jose,
Please do not top-post; I use top-posting as a tell-tale for mails I can safely delete unread when I have too many mails in my inbox.
On 2015-08-28 08:37, Jose F. Morales wrote:
> Ops... my fault. I was playing with the web editor and forgot that my
> profile didn't had my real name (now it has).
Great!
> Could I still amend the commit? (it seems to be already pushed into master)
It was pushed to Git for Windows' master, but here it was submitted to the Git mailing list.
Junio, would you terribly mind fixing the name on your end? Alternatively, I could try to update the Pull Request and give submitGit another chance to show just how awesome it is.

Ciao, Dscho

Johannes Schindelin· Aug 28, 2015, 09:44 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Mingw: verify both ends of the pipe () call

Hi,
On 2015-08-28 11:39, Johannes Schindelin wrote:
Show 10 quoted lines
> On 2015-08-28 08:37, Jose F. Morales wrote:
> 
>> Could I still amend the commit? (it seems to be already pushed into master)
> 
> It was pushed to Git for Windows' master, but here it was submitted to
> the Git mailing list.
> 
> Junio, would you terribly mind fixing the name on your end?
> Alternatively, I could try to update the Pull Request and give
> submitGit another chance to show just how awesome it is.
Never mind, it was way too easy to let submitGit show how excellent it is.

Ciao, Dscho

← back to recent threads