Johannes Schindelin <johannes.schinde...@gmx.de> writes: > The version of the "replace isatty() hack" that got applied to the > `maint` branch 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:
Thanks for an update. Does the above "the version taken was not updated before getting merged" mistake only apply to 'maint', or is it also true for 'master'? As a rule we only merge things to 'maint' that have already been merged to 'master', so I am guessing that the answer is yes, in which case I'd queue it on js/mingw-isatty and then merge it to next, master and maint in that order as usual. > - 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 > Also if the v3 had been reviewed and acked, it would be nice to have the acked-by around here (which I can locally do). Hannes? > Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de> > --- Thanks. > Published-As: > https://github.com/dscho/git/releases/tag/mingw-isatty-fixup-fixup-v1 > Fetch-It-Via: git fetch https://github.com/dscho/git > mingw-isatty-fixup-fixup-v1 > > compat/winansi.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/compat/winansi.c b/compat/winansi.c > index 3c9ed3cfe0..82b89ab137 100644 > --- a/compat/winansi.c > +++ b/compat/winansi.c > @@ -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). > > base-commit: 3313b78c145ba9212272b5318c111cde12bfef4a
