Re: [PATCH v9 0/10] MR4982: Load and play MIDI

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/dmime/midi.c:
> - hr = IStream_Read(stream, &length, sizeof(length), NULL);
> - if (FAILED(hr)) return hr;
> - length = GET_BE_DWORD(length);
> - TRACE("MIDI track length %lu\n", length);
> -
> - while (!is_end_of_track)
> - {
> -
>    if (!(event = calloc(1, sizeof(*event)))) return E_OUTOFMEMORY;
> -
> -
>    hr = read_midi_event(stream, event, last_status, &bytes_read);
> -
>    if (hr != S_OK) break;
> -
>    list_add_tail(&This->events, &event->entry);
> -
>    This->event_count++;
> -
>    last_status = event->status;
> -
>    is_end_of_track = event->status == 0xff && event->type == MIDI_META_END_OF_TRACK;
> -
>    event = NULL;

I would find it easier to reason about, and later easier to refactor (think splitting the loop body to a separate helper), if the event variable was scoped here, and either freed or inserted in the list within the loop.