2013/1/10 Johannes Kroll <jkroll@lavabit.com>
On Thu, 10 Jan 2013 01:05:28 +0100
Johannes Kroll <jkroll@lavabit.com> wrote:

> On Thu, 10 Jan 2013 00:38:06 +0100
> Christian Costa <titan.costa@gmail.com> wrote:
>
>
> > After a better look the changes seem correct.
> > The condition i < lpMidioutHdr->dwBufferLength-1 is not necessary tough.
>
> Without the buffer length check, there would be a buffer overflow when
> an application tries to send a SysEx without an F7 byte.

I meant a read past the end of the buffer of course, not an overflow.



In your code you stop checking F7 just before the last byte (until position lpMidiOutHdr->dwBufferLength-2);
You can check the last byte as well. This is also valid and simplify the condition.
The missing F7 byte case is handled by the for loop.

Regarding the single MMDRV_Message call, I was thinking about something like below.
This also enable printing a WARN when F7 is missing which can be useful for debug.

    DWORD oldBufferLength = lpMidiOutHdr->dwBufferLength;
    DWORD ret;
    for(i = 0; i < lpMidiOutHdr->dwBufferLength; i++)
       /* SysEx messages are terminated by a 0xF7 byte. If the buffer contains additional 
          bytes, send only the bytes up to the termination byte. */
        if((unsigned char)lpMidiOutHdr->lpData[i] == 0xF7)
        {
            lpMidiOutHdr->dwBufferLength = i+1;
            break;
        }



    if (i == lpMidiOutHdr->dwBufferLength)
        WARN("SysEx termination byte 0xF7 missing\n")
 
    ret =  MMDRV_Message(wmld, MODM_LONGDATA, (DWORD_PTR)lpMidiOutHdr, uSize);

    /* restore the midi header to its original state. */
    lpMidiOutHdr->dwBufferLength = oldBufferLength;

    return ret;