Thread (22 messages) 22 messages, 3 authors, 2017-01-18

Re: [PATCH v2 3/3] mingw: replace isatty() hack

flat view

From: Johannes Sixt <hidden>
Date: 2016-12-22 20:26:54

I've only one request for clarification below. Otherwise, the patch looks good.

(lines removed by the patch trimmed)

Am 22.12.2016 um 18:09 schrieb Johannes Schindelin:
quoted hunk ↗ jump to hunk
+static HANDLE swap_osfhnd(int fd, HANDLE new_handle)
+{
+	/*
+	 * Create a copy of the original handle associated with fd
+	 * because the original will get closed when we dup2().
+	 */
+	HANDLE handle = (HANDLE)_get_osfhandle(fd);
+	HANDLE duplicate = duplicate_handle(handle);

+	/* Create a temp fd associated with the already open "new_handle". */
+	int new_fd = _open_osfhandle((intptr_t)new_handle, O_BINARY);

+	assert((fd == 1) || (fd == 2));

+	/*
+	 * Use stock dup2() to re-bind fd to the new handle.  Note that
+	 * this will implicitly close(1) and close both fd=1 and the
+	 * originally associated handle.  It will open a new fd=1 and
+	 * call DuplicateHandle() on the handle associated with new_fd.
+	 * 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.
Certainly, the OS does not recycle handle values that are in use (open). Then I do not quite get the point of this paragraph. See...
quoted hunk ↗ jump to hunk
+	 *
+	 * 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;
... This is where "the cached value of console is updated", right? If console == handle, then this is not because a handle value was recycled, but because fd *was* console. Since the old value of console has been closed by the dup2(), we must refer to the back-up value in the future. Am I missing something?
quoted hunk ↗ jump to hunk
+	handle = INVALID_HANDLE_VALUE;

+	/* Close the temp fd.  This explicitly closes "new_handle"
+	 * (because it has been associated with it).
+	 */
+	close(new_fd);

+	fd_is_interactive[fd] |= FD_SWAPPED;

+	return duplicate;
 }
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help