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

Commit f1927435 authored by Atneya Nair's avatar Atneya Nair
Browse files

Fix use after move issue in AppOpsSession 

In the ctor, we were accessing the already moved from  param in the
subsequent member initializers. Fix this.

There is also an issue with implicitly converting the return value.

Test: atest AppOpsSessionTests
Bug: 355498020
Flag: EXEMPT, safe adding utils
Change-Id: I5ab4b2ecdc72a049183af3be0344886ecc7db33a
parent de8afe4c

media/libaudiopermission/AppOpsSession.cpp

+7 −6
Original line number Diff line number Diff line
@@ -35,24 +35,25 @@ void DefaultAppOpsFacade::OpMonitor::opChanged(int32_t, const String16&) { 


bool DefaultAppOpsFacade::startAccess(const ValidatedAttributionSourceState& attr_, Ops ops) {

    const AttributionSourceState& attr = attr_;

    // TODO no support for additional op at the moment

    // TODO(b/384845037) no support for additional op at the moment

    if (ops.attributedOp == AppOpsManager::OP_NONE) return true;  // nothing to do

    // TODO caching and sync up-call marking

    // TODO(b/384845037) caching and sync up-call marking

    AppOpsManager ap{};

    return ap.startOpNoThrow(

        /*op=*/ ops.attributedOp,

        /*uid=*/ attr.uid,

        /*packageName=*/ String16{attr.packageName.value_or("").c_str()},

        /*startModeIfDefault=*/ false,

        /*callingPackage=*/ String16{attr.packageName.value_or("").c_str()},

        /*startIfModeDefault=*/ false,

        /*attributionTag=*/ attr.attributionTag.has_value() ?

            String16{attr.attributionTag.value().c_str()}

                                            : String16{},

        /*msg=*/ String16{"AppOpsSession start"});

        /*message=*/ String16{"AppOpsSession start"})

    == AppOpsManager::MODE_ALLOWED;

}



void DefaultAppOpsFacade::stopAccess(const ValidatedAttributionSourceState& attr_, Ops ops) {

    const AttributionSourceState& attr = attr_;

    // TODO caching and sync up-call marking

    // TODO(b/384845037) caching and sync up-call marking

    AppOpsManager ap{};

    return ap.finishOp(

        /*op=*/ ops.attributedOp,

media/libaudiopermission/include/media/AppOpsSession.h

+5 −4
Original line number Diff line number Diff line
@@ -20,11 +20,12 @@ 
#include <binder/IAppOpsCallback.h>

#include <cutils/android_filesystem_config.h>

#include <log/log.h>

#include <media/ValidatedAttributionSourceState.h>

#include <utils/RefBase.h>



#include <functional>



#include "media/ValidatedAttributionSourceState.h"



namespace android::media::permission {



using ValidatedAttributionSourceState =
@@ -99,10 +100,10 @@ class AppOpsSession { 
          mOps(ops),

          mCb(std::move(opChangedCb)),

          mAppOps(std::move(appOpsFacade)),

          mCookie(mAppOps.addChangeCallback(attr, ops,

          mCookie(mAppOps.addChangeCallback(mAttr, ops,

                                            [this](bool x) { this->onPermittedChanged(x); })),

          mDeliveryRequested(false),

          mDeliveryPermitted(mAppOps.checkAccess(attr, ops)) {}

          mDeliveryPermitted(mAppOps.checkAccess(mAttr, ops)) { }



    ~AppOpsSession() {

        endDeliveryRequest();
@@ -188,7 +189,7 @@ class DefaultAppOpsFacade { 
    class OpMonitor : public BnAppOpsCallback {

      public:

        OpMonitor(ValidatedAttributionSourceState attr, Ops ops, std::function<void(bool)> cb)

            : mAttr(attr), mOps(ops), mCb(cb) {}

            : mAttr(std::move(attr)), mOps(ops), mCb(std::move(cb)) { }



        void opChanged(int32_t op, const String16& packageName) override;

media/libaudiopermission/tests/AppOpsSessionTests.cpp

+1 −1
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ 
 * limitations under the License.

 */



#include <android/content/AttributionSourceState.h>

#include <media/AppOpsSession.h>

#include <media/ValidatedAttributionSourceState.h>
@@ -28,7 +29,6 @@ using ::android::media::permission::Ops; 
using ::com::android::media::permission::ValidatedAttributionSourceState;



using ::testing::ElementsAreArray;

using ::testing::Eq;

using ::testing::IsEmpty;

using ::testing::Ne;