Re: [Qemu-block] [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
On Fri, Jun 30, 2017 at 3:37 AM, Paolo Bonzini wrote:
>
>
> On 29/06/2017 18:37, Alistair Francis wrote:
>>> Hmm, I think it's possible, poll_msgs is true here.
>> poll_msgs?
>>
>> If nhandles is 0 then we have already entered an earlier if statement
>> and set ready to either WAIT_FAILED or WAIT_TIMEOUT in which case we
>> can't enter this part of the if statement.
>
> No, that's not correct. The code is:
>
> if (poll_msgs) {
> /* Wait for either messages or handles
> * -> Use MsgWaitForMultipleObjectsEx
> */
> ready = MsgWaitForMultipleObjectsEx(nhandles, handles, timeout,
> QS_ALLINPUT, MWMO_ALERTABLE);
>
> if (ready == WAIT_FAILED) {
> gchar *emsg = g_win32_error_message(GetLastError());
> g_warning("MsgWaitForMultipleObjectsEx failed: %s", emsg);
> g_free(emsg);
> }
> } else if (nhandles == 0) {
> /* No handles to wait for, just the timeout */
> if (timeout == INFINITE) {
> ready = WAIT_FAILED;
> } else {
> SleepEx(timeout, TRUE);
> ready = WAIT_TIMEOUT;
> }
>
> You can have poll_msgs == TRUE && nhandles == 0. This happens for
>
>GPollFD fds[1] = { .fd = G_WIN32_MSG_HANDLE, .events = G_IO_IN };
>g_poll(fds, 1, timeout);
Ah. Yeah good point.
Ok, I'll respin the series without this patch then.
Thanks,
Alistair
>
> Thanks,
>
> Paolo
>
Re: [Qemu-block] [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
On 29/06/2017 18:37, Alistair Francis wrote:
>> Hmm, I think it's possible, poll_msgs is true here.
> poll_msgs?
>
> If nhandles is 0 then we have already entered an earlier if statement
> and set ready to either WAIT_FAILED or WAIT_TIMEOUT in which case we
> can't enter this part of the if statement.
No, that's not correct. The code is:
if (poll_msgs) {
/* Wait for either messages or handles
* -> Use MsgWaitForMultipleObjectsEx
*/
ready = MsgWaitForMultipleObjectsEx(nhandles, handles, timeout,
QS_ALLINPUT, MWMO_ALERTABLE);
if (ready == WAIT_FAILED) {
gchar *emsg = g_win32_error_message(GetLastError());
g_warning("MsgWaitForMultipleObjectsEx failed: %s", emsg);
g_free(emsg);
}
} else if (nhandles == 0) {
/* No handles to wait for, just the timeout */
if (timeout == INFINITE) {
ready = WAIT_FAILED;
} else {
SleepEx(timeout, TRUE);
ready = WAIT_TIMEOUT;
}
You can have poll_msgs == TRUE && nhandles == 0. This happens for
GPollFD fds[1] = { .fd = G_WIN32_MSG_HANDLE, .events = G_IO_IN };
g_poll(fds, 1, timeout);
Thanks,
Paolo
Re: [Qemu-block] [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
On Thu, Jun 29, 2017 at 6:32 AM, Paolo Bonzini wrote:
> On 28/06/2017 01:57, Alistair Francis wrote:
>> There is no way nhandles can be zero in this section so that part of the
>> if statement will always be false. Let's just remove it to make the code
>> easier to read.
>>
>> Signed-off-by: Alistair Francis
>> Acked-by: Edgar E. Iglesias
>> ---
>>
>> util/oslib-win32.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>> index 80e4668935..7ec0f8e083 100644
>> --- a/util/oslib-win32.c
>> +++ b/util/oslib-win32.c
>> @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE
>> *handles, gint nhandles,
>> /* If we have a timeout, or no handles to poll, be satisfied
>> * with just noticing we have messages waiting.
>> */
>> -if (timeout != 0 || nhandles == 0) {
>> +if (timeout != 0) {
>> return 1;
>> }
>>
>>
>
> Hmm, I think it's possible, poll_msgs is true here.
poll_msgs?
If nhandles is 0 then we have already entered an earlier if statement
and set ready to either WAIT_FAILED or WAIT_TIMEOUT in which case we
can't enter this part of the if statement.
Thanks,
Alistair
>
> Paolo
Re: [Qemu-block] [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
On 28/06/2017 01:57, Alistair Francis wrote:
> There is no way nhandles can be zero in this section so that part of the
> if statement will always be false. Let's just remove it to make the code
> easier to read.
>
> Signed-off-by: Alistair Francis
> Acked-by: Edgar E. Iglesias
> ---
>
> util/oslib-win32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 80e4668935..7ec0f8e083 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles,
> gint nhandles,
> /* If we have a timeout, or no handles to poll, be satisfied
> * with just noticing we have messages waiting.
> */
> -if (timeout != 0 || nhandles == 0) {
> +if (timeout != 0) {
> return 1;
> }
>
>
Hmm, I think it's possible, poll_msgs is true here.
Paolo
Re: [Qemu-block] [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
On Tue, Jun 27, 2017 at 8:57 PM, Alistair Francis
wrote:
> There is no way nhandles can be zero in this section so that part of the
> if statement will always be false. Let's just remove it to make the code
> easier to read.
>
> Signed-off-by: Alistair Francis
> Acked-by: Edgar E. Iglesias
Reviewed-by: Philippe Mathieu-Daudé
> ---
>
> util/oslib-win32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 80e4668935..7ec0f8e083 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles,
> gint nhandles,
> /* If we have a timeout, or no handles to poll, be satisfied
> * with just noticing we have messages waiting.
> */
> -if (timeout != 0 || nhandles == 0) {
> +if (timeout != 0) {
> return 1;
> }
>
> --
> 2.11.0
>
>
