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)
...
