[ruby-dev:8728] Re: [mswin32] STDERR does not work during `_function.

小松です。

In the message of [ruby-dev:8716] Re: [mswin32] STDERR does not work during `_function. 
    on 1999/12/24 17:10:30 KIMURA Koichi <[e-mail]> wrote:
|>元からですが、Processハンドルがリークしています。
|
|あ痛、お手数かけます。
|
|>あと、エラー時のCloseHandle()の処理とfree()の処理が不十分。
|># これも元からかな？
|
|多分そうです。試行錯誤の名残といえば名残ですが…

あと、たとえば、CreateProcess()でコマンドが見つからなくて失敗した
場合など、rb_fatal()は強すぎると思います。
また、malloc()はxmalloc()に変更しました。

ということで、こんなところかな？

# ts=8 sw=4にしたいという誘惑はなんとか我慢しました……

--- io.c.dist	Tue Dec 14 15:50:38 1999
+++ io.c	Fri Dec 24 15:11:15 1999
@@ -1449,5 +1449,6 @@ pipe_open(pname, mode)
 	if (modef & FMODE_READABLE) fptr->f  = f;
 	if (modef & FMODE_WRITABLE) {
-	    fptr->f2 = f;
+	    if (fptr->f) fptr->f2 = f;
+	    else fptr->f = f;
 	    rb_io_synchronized(fptr);
 	}
--- win32/win32.c.dist	Tue Nov  9 12:57:28 1999
+++ win32/win32.c	Sun Dec 26 23:15:13 1999
@@ -230,5 +230,4 @@ struct {
     int inuse;
     int pid;
-    HANDLE oshandle;
     FILE *pipe;
 } MyPopenRecord[MYPOPENSIZE];
@@ -496,5 +495,5 @@ mypopen (char *cmd, char *mode) 
 
 		BOOL fRet;
-		HANDLE hInFile, hOutFile, hStdin, hStdout;
+		HANDLE hInFile, hOutFile;
 		LPCSTR lpApplicationName = NULL;
 		LPTSTR lpCommandLine;
@@ -510,21 +509,9 @@ mypopen (char *cmd, char *mode) 
 		sa.bInheritHandle       = TRUE;
 
-		if (!reading) {
-        	FILE *fp;
-
-			fp = (_popen)(cmd, mode);
-
-			MyPopenRecord[slot].inuse = TRUE;
-			MyPopenRecord[slot].pipe = fp;
-			MyPopenRecord[slot].pid = -1;
-
-			if (!fp)
-                        rb_fatal("cannot open pipe \"%s\" (%s)", cmd, strerror(errno));
-				return fp;
-		}
-
 		fRet = CreatePipe(&hInFile, &hOutFile, &sa, 2048L);
-		if (!fRet)
-                        rb_fatal("cannot open pipe \"%s\" (%s)", cmd, strerror(errno));
+		if (!fRet) {
+			errno = GetLastError();
+			rb_sys_fail("mypopen: CreatePipe");
+		}
 
 		memset(&aStartupInfo, 0, sizeof (STARTUPINFO));
@@ -534,33 +521,12 @@ mypopen (char *cmd, char *mode) 
 
 		if (reading) {
-			aStartupInfo.hStdInput  = GetStdHandle(STD_OUTPUT_HANDLE);//hStdin;
-			aStartupInfo.hStdError  = INVALID_HANDLE_VALUE;
-			//for save
-			DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_OUTPUT_HANDLE),
-			  GetCurrentProcess(), &hStdout,
-			  0, FALSE, DUPLICATE_SAME_ACCESS
-			);
-			//for redirect
-			DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_INPUT_HANDLE),
-			  GetCurrentProcess(), &hStdin,
-			  0, TRUE, DUPLICATE_SAME_ACCESS
-			);
+			aStartupInfo.hStdInput  = GetStdHandle(STD_INPUT_HANDLE);
 			aStartupInfo.hStdOutput = hOutFile;
 		}
 		else {
-			aStartupInfo.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE); //hStdout;
-			aStartupInfo.hStdError  = INVALID_HANDLE_VALUE;
-			// for save
-			DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_INPUT_HANDLE),
-			  GetCurrentProcess(), &hStdin,
-			  0, FALSE, DUPLICATE_SAME_ACCESS
-			);
-			//for redirect
-			DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_OUTPUT_HANDLE),
-			  GetCurrentProcess(), &hStdout,
-			  0, TRUE, DUPLICATE_SAME_ACCESS
-			);
-			aStartupInfo.hStdInput = hInFile;
+			aStartupInfo.hStdInput  = hInFile;
+			aStartupInfo.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
 		}
+		aStartupInfo.hStdError  = GetStdHandle(STD_ERROR_HANDLE);
 
 		dwCreationFlags = (NORMAL_PRIORITY_CLASS);
@@ -569,7 +535,5 @@ mypopen (char *cmd, char *mode) 
 		if (NtHasRedirection(cmd) || isInternalCmd(cmd)) {
 		  lpApplicationName = getenv("COMSPEC");
-		  lpCmd2 = malloc(strlen(lpApplicationName) + 1 + strlen(cmd) + sizeof (" /c "));
-		  if (lpCmd2 == NULL)
-                     rb_fatal("Mypopen: malloc failed");
+		  lpCmd2 = xmalloc(strlen(lpApplicationName) + 1 + strlen(cmd) + sizeof (" /c "));
 		  sprintf(lpCmd2, "%s %s%s", lpApplicationName, " /c ", cmd);
 		  lpCommandLine = lpCmd2;
@@ -578,47 +542,41 @@ mypopen (char *cmd, char *mode) 
 		fRet = CreateProcess(lpApplicationName, lpCommandLine, &sa, &sa,
 			sa.bInheritHandle, dwCreationFlags, NULL, NULL, &aStartupInfo, &aProcessInformation);
+		errno = GetLastError();
+
+		if (lpCmd2)
+			free(lpCmd2);
+
+		CloseHandle(aProcessInformation.hThread);
 
 		if (!fRet) {
 			CloseHandle(hInFile);
 			CloseHandle(hOutFile);
-                        rb_fatal("cannot fork for \"%s\" (%s)", cmd, strerror(errno));
+			CloseHandle(aProcessInformation.hProcess);
+			return NULL;
 		}
 
-		CloseHandle(aProcessInformation.hThread);
-
 		if (reading) {
-			HANDLE hDummy;
-
 			fd = _open_osfhandle((long)hInFile,  (_O_RDONLY | pipemode));
 			CloseHandle(hOutFile);
-			DuplicateHandle(GetCurrentProcess(), hStdout,
-			  GetCurrentProcess(), &hDummy,
-			  0, TRUE, (DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)
-			);
 		}
 		else {
-			HANDLE hDummy;
-
-		    fd = _open_osfhandle((long)hOutFile, (_O_WRONLY | pipemode));
+			fd = _open_osfhandle((long)hOutFile, (_O_WRONLY | pipemode));
 			CloseHandle(hInFile);
-			DuplicateHandle(GetCurrentProcess(), hStdin,
-			  GetCurrentProcess(), &hDummy,
-			  0, TRUE, (DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)
-			);
 		}
 
-		if (fd == -1) 
-                  rb_fatal("cannot open pipe \"%s\" (%s)", cmd, strerror(errno));
-
-
-		if ((fp = (FILE *) fdopen(fd, mode)) == NULL)
-			return NULL;
+		if (fd == -1) {
+			CloseHandle(reading ? hInFile : hOutFile);
+			CloseHandle(aProcessInformation.hProcess);
+			rb_sys_fail("mypopen: _open_osfhandle");
+		}
 
-		if (lpCmd2)
-			free(lpCmd2);
+		if ((fp = (FILE *) fdopen(fd, mode)) == NULL) {
+			_close(fd);
+			CloseHandle(aProcessInformation.hProcess);
+			rb_sys_fail("mypopen: fdopen");
+		}
 
 		MyPopenRecord[slot].inuse = TRUE;
 		MyPopenRecord[slot].pipe  = fp;
-		MyPopenRecord[slot].oshandle = (reading ? hInFile : hOutFile);
 		MyPopenRecord[slot].pid   = (int)aProcessInformation.hProcess;
 		return fp;
@@ -672,12 +630,11 @@ mypclose(FILE *fp)
 		}
 	}
+	CloseHandle((HANDLE)MyPopenRecord[i].pid);
 #endif
 
-
     //
     // close the pipe
     //
-    // Closehandle() is done by fclose().
-    //CloseHandle(MyPopenRecord[i].oshandle);
+
     fflush(fp);
     fclose(fp);

--
小松克行 / Katsuyuki Komatsu <[e-mail]>