Re: [PATCH] winecoreaudio.drv: Only read 'length' bytes from received MIDIPackets.

[Andrew Eikum]

Signed-off-by: Andrew Eikum

---
I just reviewed the code by eye, I don't have a macOS environment set
up.

On Wed, Oct 28, 2020 at 05:29:13PM -0700, Brendan Shanks wrote:
> The Core MIDI MIDIPacket struct is declared with a 256-byte data array,
> but this is just for convenience. There may be only one accessible byte,
> or there may be more than 256.
> Only read 'length' bytes from the packet, and correctly handle a
> length > 256.
> Signed-off-by: Brendan Shanks
>
> The previous code was also passing a 260 byte struct by value, and
> copying around 256 bytes even though most MIDI messages are only a few
> bytes.
>
> dlls/winecoreaudio.drv/coremidi.c |  7 ++-----
>  dlls/winecoreaudio.drv/coremidi.h |  8 +-------
>  dlls/winecoreaudio.drv/midi.c     | 18 +++++++++++++++---
>  3 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/dlls/winecoreaudio.drv/coremidi.c b/dlls/winecoreaudio.drv/coremidi.c
> index 5ec564603a9..94761b05c4f 100644
> --- a/dlls/winecoreaudio.drv/coremidi.c
> +++ b/dlls/winecoreaudio.drv/coremidi.c
> @@ -57,15 +57,12 @@ void CoreMIDI_GetObjectName(MIDIObjectRef obj, char *name, int size)
>  void MIDIIn_ReadProc(const MIDIPacketList *pktlist, void *refCon, void *connRefCon)
>  {
>      unsigned int i;
> -
>
> MIDIMessage msg;
>
> MIDIPacket *packet = (MIDIPacket *)pktlist->packet;
> for (i = 0; i < pktlist->numPackets; ++i) {
> -
>    msg.devID = *((UInt16 *)connRefCon);
> -
>    msg.length = packet->length;
> -
>    memcpy(msg.data, packet->data, sizeof(packet->data));
> -
>    UInt16 devID = *((UInt16 *)connRefCon);
>
> -
>    MIDIIn_SendMessage(msg);
> -
>    MIDIIn_SendMessage(devID, packet->data, packet->length);
>
>    packet = MIDIPacketNext(packet);
>
> }
>
> diff --git a/dlls/winecoreaudio.drv/coremidi.h b/dlls/winecoreaudio.drv/coremidi.h
> index c93edc6c31d..3433a1d4342 100644
> --- a/dlls/winecoreaudio.drv/coremidi.h
> +++ b/dlls/winecoreaudio.drv/coremidi.h
> @@ -65,12 +65,6 @@ extern int AudioUnit_SetVolume(AudioUnit au, float left, float right);
>  extern int AudioUnit_GetVolume(AudioUnit au, float *left, float *right);
>  #endif
>
> -typedef struct {
> - UInt16 devID;
> - UInt16 length;
> - Byte data[256];
>
> -} MIDIMessage;
> -
>
> /* coremidi.c */
>  extern MIDIClientRef CoreMIDI_CreateClient(CFStringRef name);
>  extern void CoreMIDI_GetObjectName(MIDIObjectRef obj, char *name, int size);
> @@ -79,6 +73,6 @@ extern void MIDIIn_ReadProc(const MIDIPacketList *pktlist, void *refCon, void *c
>  extern void MIDIOut_Send(MIDIPortRef port, MIDIEndpointRef dest, UInt8 *buffer, unsigned length);
>
> /* midi.c */
> -void MIDIIn_SendMessage(MIDIMessage msg);
> +void MIDIIn_SendMessage(UInt16 devID, const void *buffer, UInt16 length);
>
> #endif
> diff --git a/dlls/winecoreaudio.drv/midi.c b/dlls/winecoreaudio.drv/midi.c
> index edaf1283124..c299d116f12 100644
> --- a/dlls/winecoreaudio.drv/midi.c
> +++ b/dlls/winecoreaudio.drv/midi.c
> @@ -78,6 +78,12 @@ typedef struct tagMIDISource {
>      DWORD startTime;
>  } MIDISource;
>
> +typedef struct {
> - UInt16 devID;
> - UInt16 length;
> - Byte data[];
>
> +} MIDIMessage;
> -
>
> static CRITICAL_SECTION midiInLock; /* Critical section for MIDI In */
>  static CFStringRef MIDIInThreadPortName = NULL;
>
> @@ -797,16 +803,22 @@ static DWORD MIDIIn_Reset(WORD wDevID)
> - Call from CoreMIDI IO threaded callback,
> - we can't call Wine debug channels, critical section or anything using NtCurrentTeb here.
>
> */
> -void MIDIIn_SendMessage(MIDIMessage msg)
> +void MIDIIn_SendMessage(UInt16 devID, const void *buffer, UInt16 length)
>  {
> - CFDataRef data;
> -
>
> MIDIMessage msg;
> -
>
> CFMutableDataRef data;
>
> CFMessagePortRef messagePort;
> messagePort = CFMessagePortCreateRemote(kCFAllocatorDefault, MIDIInThreadPortName);
> - data = CFDataCreate(kCFAllocatorDefault, (UInt8 *) &msg, sizeof(msg));
> - msg.devID = devID;
> - msg.length = length;
> -
> - data = CFDataCreateMutable(kCFAllocatorDefault, sizeof(msg) + length);
> if (data)
> {
> -
>    CFDataAppendBytes(data, (UInt8 *) &msg, sizeof(msg));
> -
>    CFDataAppendBytes(data, buffer, length);
>    CFMessagePortSendRequest(messagePort, 0, data, 0.0, 0.0, NULL, NULL);
>    CFRelease(data);
> }
>
> -- 
> 2.26.2