[webkit-reviews] review granted: [Bug 169956] [Crash] WebCore::AudioBuffer::AudioBuffer don't checking illegal value : [Attachment 305487] Updated patch.

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 169956: [Crash] WebCore::AudioBuffer::AudioBuffer don't checking illegal
value
https://bugs.webkit.org/show_bug.cgi?id=169956

Attachment 305487: Updated patch.

https://bugs.webkit.org/attachment.cgi?id=305487&action=review




--- Comment #11 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 305487
  --> https://bugs.webkit.org/attachment.cgi?id=305487
Updated patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=305487&action=review

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:47
> +    RefPtr<AudioBuffer> buffer = adoptRef(*new AudioBuffer(numberOfChannels,
numberOfFrames, sampleRate));

Should be auto or Ref<>.

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:48
> +    if (!buffer || !buffer->m_length)

Should just be !buffer->m_length I guess.
In the case of !buffer, we are probably in a very bad situation and will crash
anyway.

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:76
>	   m_channels.append(channelDataArray);

Use WTFMove here.
Or maybe refactor to put more code in common between the constructors, like an
allocateChannelDataArray method that would be called fromAudioBuffer::create.

> Source/WebCore/Modules/webaudio/AudioBuffer.cpp:95
>	   channelDataArray->setRange(bus.channel(i)->data(), m_length, 0);

Why are we setting the range here but not in the other constructor?