Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 663c2247 authored by Glenn Kasten's avatar Glenn Kasten
Browse files

Consistent error checking for sp<IMemory> and pointer() 

There have been concerns that an sp<IMemory> could be non-0, but the
associated pointer() still be NULL.  There are rumors this may happen
when a non-0 sp<IMemory> is passed in by client but the shared memory cannot
be re-mapped into mediaserver.

There's also evidence in the early (2009/03/03) pre-git code of checking
pointer() for NULL, after a local allocate() returned a non-0 sp<IMemory>.
It's not clear if this is "cargo cult" paranoia, or if there was a
genuine reason for the check.

In any case, we now consistently check pointer() for sp<IMemory>
input parameters in createTrack() and queueTimedBuffer().

We also check after successful allocate().  If allocate() returns a
non-0 sp<> but NULL pointer(), then treat it as if the allocate() had
returned 0.

Change-Id: I3013ac5766b493d443ecef71711ec861076a623e
parent 30ff92cb

services/audioflinger/AudioFlinger.cpp

+6 −0
Original line number Diff line number Diff line
@@ -476,6 +476,12 @@ sp<IAudioTrack> AudioFlinger::createTrack( 
        goto Exit;

    }



    if (sharedBuffer != 0 && sharedBuffer->pointer() == NULL) {

        ALOGE("createTrack() sharedBuffer is non-0 but has NULL pointer()");

        lStatus = BAD_VALUE;

        goto Exit;

    }



    {

        Mutex::Autolock _l(mLock);

        PlaybackThread *thread = checkPlaybackThread_l(output);

services/audioflinger/Tracks.cpp

+9 −5
Original line number Diff line number Diff line
@@ -116,12 +116,11 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase( 


    if (client != 0) {

        mCblkMemory = client->heap()->allocate(size);

        if (mCblkMemory != 0) {

            mCblk = static_cast<audio_track_cblk_t *>(mCblkMemory->pointer());

            // can't assume mCblk != NULL

        } else {

        if (mCblkMemory == 0 ||

                (mCblk = static_cast<audio_track_cblk_t *>(mCblkMemory->pointer())) == NULL) {

            ALOGE("not enough memory for AudioTrack size=%u", size);

            client->heap()->dump("AudioTrack");

            mCblkMemory.clear();

            return;

        }

    } else {
@@ -275,6 +274,11 @@ status_t AudioFlinger::TrackHandle::queueTimedBuffer(const sp<IMemory>& buffer, 
    if (!mTrack->isTimedTrack())

        return INVALID_OPERATION;



    if (buffer == 0 || buffer->pointer() == NULL) {

        ALOGE("queueTimedBuffer() buffer is 0 or has NULL pointer()");

        return BAD_VALUE;

    }



    PlaybackThread::TimedTrack* tt =

            reinterpret_cast<PlaybackThread::TimedTrack*>(mTrack.get());

    return tt->queueTimedBuffer(buffer, pts);
@@ -1060,7 +1064,7 @@ status_t AudioFlinger::PlaybackThread::TimedTrack::allocateTimedBuffer( 
    }



    sp<IMemory> newBuffer = mTimedMemoryDealer->allocate(size);

    if (newBuffer == 0) {

    if (newBuffer == 0 || newBuffer->pointer() == NULL) {

        return NO_MEMORY;

    }