OSDN Git Service

mingw: follow-up to "replace isatty() hack"
authorJohannes Schindelin <johannes.schindelin@gmx.de>
Wed, 18 Jan 2017 12:14:35 +0000 (13:14 +0100)
committerJunio C Hamano <gitster@pobox.com>
Wed, 18 Jan 2017 21:31:25 +0000 (13:31 -0800)
The version of the "replace isatty() hack" that got merged a few
weeks ago did not actually reflect the latest iteration of the patch
series: v3 was sent out with these changes, as requested by the
reviewer Johannes Sixt:

- reworded the comment about "recycling handles"

- moved the reassignment of the `console` variable before the dup2()
  call so that it is valid at all times

- removed the "handle = INVALID_HANDLE_VALUE" assignment, as the local
  variable `handle` is not used afterwards anyway

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
compat/winansi.c

index 477209f..56658ec 100644 (file)
@@ -494,19 +494,16 @@ static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
         * It is because of this implicit close() that we created the
         * copy of the original.
         *
-        * Note that the OS can recycle HANDLE (numbers) just like it
-        * recycles fd (numbers), so we must update the cached value
-        * of "console".  You can use GetFileType() to see that
-        * handle and _get_osfhandle(fd) may have the same number
-        * value, but they refer to different actual files now.
+        * Note that we need to update the cached console handle to the
+        * duplicated one because the dup2() call will implicitly close
+        * the original one.
         *
         * Note that dup2() when given target := {0,1,2} will also
         * call SetStdHandle(), so we don't need to worry about that.
         */
-       dup2(new_fd, fd);
        if (console == handle)
                console = duplicate;
-       handle = INVALID_HANDLE_VALUE;
+       dup2(new_fd, fd);
 
        /* Close the temp fd.  This explicitly closes "new_handle"
         * (because it has been associated with it).