Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.

[Alexandre Julliard]

[e-mail] writes:
> @@ -1006,7 +1008,9 @@ static	BOOL	MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
>      LPMIDIHDR	lpMidiHdr;
>      LPMIDIHDR*	lpmh;
>      LPBYTE	lpData;
> - BOOL	paused = FALSE;
>
> +still_paused:
>      switch (msg->message) {
>      case WM_QUIT:
>    SetEvent(lpMidiStrm->hEvent);
> @@ -1028,6 +1032,16 @@ static	BOOL	MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
>    }
>    lpMidiStrm->lpMidiHdr = 0;
>    SetEvent(lpMidiStrm->hEvent);
> - return TRUE;
> - case WINE_MSM_RESUME:
> - /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */
> - lpMidiStrm->dwStartTicks = GetTickCount() - lpMidiStrm->dwPositionMS;
> - SetEvent(lpMidiStrm->hEvent);
> - return TRUE;
> - case WINE_MSM_PAUSE:
> - /* FIXME: send out cc64 0 (turn off sustain pedal) on every channel */
> - paused = TRUE;
> - SetEvent(lpMidiStrm->hEvent);
> break;
> case WINE_MSM_HEADER:
> /* sets initial tick count for first MIDIHDR */
>
> @@ -1114,6 +1128,10 @@ static	BOOL	MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
>    FIXME("Unknown message %d\n", msg->message);
>    break;
>      }
> - if (paused) {
> - GetMessageA(msg, 0, 0, 0);
> - goto still_paused;
> - }

Please stop abusing goto, write a proper loop instead.

[PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.

Alexandre Julliard wrote:
> Please stop abusing goto, write a proper loop instead.

No problem. The idea was to minimize the patch and show what really changed.
Adding a while loop will indent all 120 lines of code in between { }.

Regards,
    Jörg Höhle

[PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.

[Michael Stefaniuc]

[e-mail] wrote:
> Alexandre Julliard wrote:
> > Please stop abusing goto, write a proper loop instead.
>
> No problem. The idea was to minimize the patch and show what really changed.
> Adding a while loop will indent all 120 lines of code in between { }.

See that as an opportunity to clean up the whitespace issues in those
120 lines ;). Or of course to split the code out into helper functions.

bye
    michael

[PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.

Michael Stefaniuc wrote:
> See that as an opportunity to clean up the whitespace issues in those 120 lines ;)

Hmm, what was the latest word on reformatting, if any?
Until now I've been careful to preserve the surrounding formatting, even
Though winmm&mci look very different from mmdevapi.

Do you suggest to remove all TAB from lines that I touch, even if only re-indenting?

Regards,
    Jörg Höhle

[PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.

[Michael Stefaniuc]

[e-mail] wrote:
> Michael Stefaniuc wrote:
> > See that as an opportunity to clean up the whitespace issues in those 120 lines ;)
>
> Hmm, what was the latest word on reformatting, if any?
> Until now I've been careful to preserve the surrounding formatting, even
> Though winmm&mci look very different from mmdevapi.
>
> Do you suggest to remove all TAB from lines that I touch, even if only re-indenting?

That. And getting rid of trailing/dangling whitespace, placing the
whitespace consistently around operators, etc.

bye
    michael