ok, if you think that using orig_stderr is bad idea (either NULL or INVALID_HANDLE_VALUE are special), maybe we should assign special variable which will keep an evidence whether original stderr was redirected or not and perform check against that variable ? like
BOOL stderr_was_redirected = 0; .... if(stderr_was_redirected) return orig_stderr; else return GetStdHandle (STD_ERROR_HANDLE); ...... /* save original stderr for password prompts */ stderr_was_redirected = 1; orig_stderr = GetStdHandle (STD_ERROR_HANDLE); 2013/12/6 Matthias Andree <matthias.and...@gmx.de>: > Am 04.12.2013 14:12, schrieb Илья Шипицин: >> Hello! >> >> I confirm issue described here: https://forums.openvpn.net/topic13246.html >> also, I confirm that it is due to random handle value (which somehow >> was null before Win 8.1 was released) >> >> also, I do not understand why we should check "err" handle on >> INVALID_HANDLE_VALUE, it will fail anyway on WriteFile, that check is >> useless. >> >> I do not insist on being perfect, if someone has better idea how to >> solve https://forums.openvpn.net/topic13246.html - I do not mind. >> >> I can make git pull request from >> https://github.com/chipitsine/openvpn/commit/3987bd0e035b653a5017c5eccbd54eaf22746852 >> if it's better. >> >> people encounter problems on Win 8.1 (well, just couple of people >> here, we are running openvpn with tens of customers on Win 8.1), I >> beleive it is serious issue, please consider release of openvpn. >> >> diff --git a/src/openvpn/console.c b/src/openvpn/console.c >> index afda8ca..3ec5758 100644 >> --- a/src/openvpn/console.c >> +++ b/src/openvpn/console.c >> @@ -62,7 +62,6 @@ get_console_input_win32 (const char *prompt, const >> bool echo, char *input, const >> err = get_orig_stderr (); >> >> if (in != INVALID_HANDLE_VALUE >> - && err != INVALID_HANDLE_VALUE >> && !win32_service_interrupt (&win32_signal) >> && WriteFile (err, prompt, strlen (prompt), &len, NULL)) >> { > > Ilya, I am sorry to have to veto this for now. > > NAK from me, this introduces a new bug. If GetStdError() inside > get_orig_stderr() fails, you will end up with err == > INVALID_HANDLE_VALUE here, so the check err != INVALID_HANDLE_VALUE must > not be removed. > > Similarly, if get_orig_stderr() returns NULL, you must not call > WriteFile() either, so how about an additional "&& err != NULL" here? > > I don't have Win 8.1 and cannot test that. > >> diff --git a/src/openvpn/error.c b/src/openvpn/error.c >> index 6848425..acc7977 100644 >> --- a/src/openvpn/error.c >> +++ b/src/openvpn/error.c >> @@ -454,12 +454,12 @@ close_syslog () >> >> #ifdef WIN32 >> >> -static HANDLE orig_stderr; >> +static HANDLE orig_stderr = INVALID_HANDLE_VALUE; >> >> HANDLE >> get_orig_stderr (void) >> { >> - if (orig_stderr) >> + if (orig_stderr != INVALID_HANDLE_VALUE) >> return orig_stderr; >> else >> return GetStdHandle (STD_ERROR_HANDLE); >> > > NAK on this, too. It exchanges one bug for another. > > The STD_ERROR_HANDLE interface has, according to > <http://msdn.microsoft.com/en-us/library/windows/desktop/ms683231%28v=vs.85%29.aspx>, > two special values, INVALID_HANDLE_VALUE (as error marker) and NULL (as > marker for "we don't have a console"). > > I think that there needs to be a separate/new "orig_stderr_is_valid" > variable, or at least the if (...) else needs to be > > if (orig_stderr != NULL && orig_stderr != INVALID_HANDLE_VALUE) > ... > > ------------------------------------------------------------------------------ > Sponsored by Intel(R) XDK > Develop, test and display web and hybrid apps with a single code base. > Download it for free now! > http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel