On: Oboe: A C++ library for low latency audio in Andro...

[evozer]

I looked at the implementation of the openStream() function:

  AudioStream *AudioStreamBuilder::build() {
      AudioStream *stream = nullptr;
      if (mAudioApi == AudioApi::AAudio && isAAudioSupported()) {
          stream = new AudioStreamAAudio(*this);  

      // If unspecified, only use AAudio if recommended.
      } else if (mAudioApi == AudioApi::Unspecified && isAAudioRecommended()) {
          stream = new AudioStreamAAudio(*this);
      } else {
          if (getDirection() == oboe::Direction::Output) {
              stream = new AudioOutputStreamOpenSLES(*this);
          } else if (getDirection() == oboe::Direction::Input) {
              stream = new AudioInputStreamOpenSLES(*this);
          }
      }
      return stream;
  }  

  Result AudioStreamBuilder::openStream(AudioStream **streamPP) {
      if (streamPP == nullptr) {
          return Result::ErrorNull;
      }
      *streamPP = nullptr;
      AudioStream *streamP = build();
      if (streamP == nullptr) {
          return Result::ErrorNull;
      }
      Result result = streamP->open(); // TODO review API
      if (result == Result::OK) {
          *streamPP = streamP;
      }
      return result;
  }

Doesn't this leak memory if streamP->open() fails? I'm also surprised they are using new instead of unique_ptr, especially since one of their listed benefits is "Convenient C++ API (uses the C++11 standard) ".

[lossolo → evozer]

Yeah, seems like memory leak to me too. streamP is leaked if open fails.

And this in modern C++ in year 2018:

if (result != Result::OK) { goto error2; }

[libso → evozer]

One the face of it, it does look like the memory pointed by streamP leaks. Unless build () kept another pointer to be used in the destructor.