From ddfe2bc5f01ff93fde1a63cbff3f233c1bc995ee Mon Sep 17 00:00:00 2001 From: Devin Anderson Date: Mon, 11 Apr 2011 23:40:25 -0700 Subject: [PATCH] Put 'alsarawmidi' Poll code inline, as it's only called in one place in the driver. Fix potential memory leak with ALSA rawmidi info. Do away with unnecessary auto_ptrs. --- linux/alsarawmidi/JackALSARawMidiDriver.cpp | 130 ++++++++++++-------- linux/alsarawmidi/JackALSARawMidiDriver.h | 7 +- 2 files changed, 82 insertions(+), 55 deletions(-) diff --git a/linux/alsarawmidi/JackALSARawMidiDriver.cpp b/linux/alsarawmidi/JackALSARawMidiDriver.cpp index d9041072..1880823c 100755 --- a/linux/alsarawmidi/JackALSARawMidiDriver.cpp +++ b/linux/alsarawmidi/JackALSARawMidiDriver.cpp @@ -145,13 +145,33 @@ JackALSARawMidiDriver::Execute() for (;;) { jack_nframes_t process_frame; unsigned short revents; - jack_nframes_t *timeout_frame_ptr; + struct timespec timeout; + struct timespec *timeout_ptr; if (! timeout_frame) { - timeout_frame_ptr = 0; + timeout_ptr = 0; } else { - timeout_frame_ptr = &timeout_frame; + + // We use a relative timeout here. By the time ppoll is called, + // the wait time is larger than it needs to be. Maybe we should + // use a timerfd instead. + // + // Also, ppoll is inefficient in certain cases. We might want to + // consider replacing it with epoll, though I'm uncertain as to + // whether or not it would be more efficient in this case. + + timeout_ptr = &timeout; + jack_time_t next_time = GetTimeFromFrames(timeout_frame); + jack_time_t now = GetMicroSeconds(); + if (next_time <= now) { + timeout.tv_sec = 0; + timeout.tv_nsec = 0; + } else { + jack_time_t wait_time = next_time - now; + timeout.tv_sec = wait_time / 1000000; + timeout.tv_nsec = (wait_time % 1000000) * 1000; + } } - if (Poll(timeout_frame_ptr) == -1) { + if (ppoll(poll_fds, poll_fd_count, timeout_ptr, 0) == -1) { if (errno == EINTR) { continue; } @@ -208,6 +228,21 @@ JackALSARawMidiDriver::Execute() return false; } +void +JackALSARawMidiDriver:: +FreeDeviceInfo(std::vector *in_info_list, + std::vector *out_info_list) +{ + size_t length = in_info_list.size(); + for (size_t i = 0; i < length; i++) { + snd_rawmidi_info_free(in_info_list.at(i)); + } + length = out_info_list.size(); + for (size_t i = 0; i < length; i++) { + snd_rawmidi_info_free(out_info_list.at(i)); + } +} + void JackALSARawMidiDriver:: GetDeviceInfo(snd_ctl_t *control, snd_rawmidi_info_t *info, @@ -321,22 +356,29 @@ JackALSARawMidiDriver::Open(bool capturing, bool playing, int in_channels, if (! (potential_inputs || potential_outputs)) { jack_error("JackALSARawMidiDriver::Open - no ALSA raw MIDI input or " "output ports found."); + FreeDeviceInfo(&in_info_list, &out_info_list); return -1; } - - // XXX: Can't use auto_ptr here. These are arrays, and require the - // delete[] operator. - std::auto_ptr input_ptr; if (potential_inputs) { - input_ports = new JackALSARawMidiInputPort *[potential_inputs]; - input_ptr.reset(input_ports); + try { + input_ports = new JackALSARawMidiInputPort *[potential_inputs]; + } catch (std::exception e) { + jack_error("JackALSARawMidiDriver::Open - while creating input " + "port array: %s", e.what()); + FreeDeviceInfo(&in_info_list, &out_info_list); + return -1; + } } - std::auto_ptr output_ptr; if (potential_outputs) { - output_ports = new JackALSARawMidiOutputPort *[potential_outputs]; - output_ptr.reset(output_ports); + try { + output_ports = new JackALSARawMidiOutputPort *[potential_outputs]; + } catch (std::exception e) { + jack_error("JackALSARawMidiDriver::Open - while creating output " + "port array: %s", e.what()); + FreeDeviceInfo(&in_info_list, &out_info_list); + goto delete_input_ports; + } } - size_t num_inputs = 0; size_t num_outputs = 0; for (size_t i = 0; i < potential_inputs; i++) { @@ -361,49 +403,33 @@ JackALSARawMidiDriver::Open(bool capturing, bool playing, int in_channels, } snd_rawmidi_info_free(info); } - if (num_inputs || num_outputs) { - if (! JackMidiDriver::Open(capturing, playing, num_inputs, num_outputs, - monitor, capture_driver_name, - playback_driver_name, capture_latency, - playback_latency)) { - if (potential_inputs) { - input_ptr.release(); - } - if (potential_outputs) { - output_ptr.release(); - } - return 0; - } - jack_error("JackALSARawMidiDriver::Open - JackMidiDriver::Open error"); - } else { + if (! (num_inputs || num_outputs)) { jack_error("JackALSARawMidiDriver::Open - none of the potential " "inputs or outputs were successfully opened."); - } - Close(); - return -1; -} - -int -JackALSARawMidiDriver::Poll(const jack_nframes_t *wakeup_frame) -{ - struct timespec timeout; - struct timespec *timeout_ptr; - if (! wakeup_frame) { - timeout_ptr = 0; + } else if (JackMidiDriver::Open(capturing, playing, num_inputs, + num_outputs, monitor, capture_driver_name, + playback_driver_name, capture_latency, + playback_latency)) { + jack_error("JackALSARawMidiDriver::Open - JackMidiDriver::Open error"); } else { - timeout_ptr = &timeout; - jack_time_t next_time = GetTimeFromFrames(*wakeup_frame); - jack_time_t now = GetMicroSeconds(); - if (next_time <= now) { - timeout.tv_sec = 0; - timeout.tv_nsec = 0; - } else { - jack_time_t wait_time = next_time - now; - timeout.tv_sec = wait_time / 1000000; - timeout.tv_nsec = (wait_time % 1000000) * 1000; + return 0; + } + if (output_ports) { + for (int i = 0; i < num_outputs; i++) { + delete output_ports[i]; } + delete[] output_ports; + output_ports = 0; } - return ppoll(poll_fds, poll_fd_count, timeout_ptr, 0); + delete_input_ports: + if (input_ports) { + for (int i = 0; i < num_inputs; i++) { + delete input_ports[i]; + } + delete[] input_ports; + input_ports = 0; + } + return -1; } int diff --git a/linux/alsarawmidi/JackALSARawMidiDriver.h b/linux/alsarawmidi/JackALSARawMidiDriver.h index 4dfa05bc..692d9cc7 100755 --- a/linux/alsarawmidi/JackALSARawMidiDriver.h +++ b/linux/alsarawmidi/JackALSARawMidiDriver.h @@ -44,6 +44,10 @@ namespace Jack { struct pollfd *poll_fds; JackThread *thread; + void + FreeDeviceInfo(std::vector *in_info_list, + std::vector *out_info_list); + void GetDeviceInfo(snd_ctl_t *control, snd_rawmidi_info_t *info, std::vector *info_list); @@ -52,9 +56,6 @@ namespace Jack { HandleALSAError(const char *driver_func, const char *alsa_func, int code); - int - Poll(const jack_nframes_t *wakeup_frame); - public: JackALSARawMidiDriver(const char *name, const char *alias,