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

Commit caa776ce authored by Steven Moreland's avatar Steven Moreland
Browse files

libbinder_ndk: finalizeTransaction->AParcel_delete 

AIBinder_finalizeTransaction has been removed and replaced with
AParcel_delete. This makes error handling easier and makes it possible
to clean up the in parcel without executing a transaction.

This comes with the loss of the check that the parcel is read in its
entirety as this check may be moved into libbinder or into an API above
this in order to simplify error handling.

Bug: 111445392
Test: runtests.sh
Change-Id: I4e0d0324f0333b02e6a47c7190819822850bee93
parent e9f06443

libs/binder/ndk/AIBinder.cpp

+2 −37
Original line number Diff line number Diff line
@@ -279,12 +279,6 @@ binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in) { 
    return status;

}



using AutoParcelDestroyer = std::unique_ptr<AParcel*, void (*)(AParcel**)>;

static void destroy_parcel(AParcel** parcel) {

    delete *parcel;

    *parcel = nullptr;

}



binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, AParcel** in,

                                  AParcel** out, binder_flags_t flags) {

    if (in == nullptr) {
@@ -292,9 +286,10 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, APa 
        return EX_NULL_POINTER;

    }



    using AutoParcelDestroyer = std::unique_ptr<AParcel*, void (*)(AParcel**)>;

    // This object is the input to the transaction. This function takes ownership of it and deletes

    // it.

    AutoParcelDestroyer forIn(in, destroy_parcel);

    AutoParcelDestroyer forIn(in, AParcel_delete);



    if (!isUserCommand(code)) {

        LOG(ERROR) << __func__ << ": Only user-defined transactions can be made from the NDK.";
@@ -329,33 +324,3 @@ binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, APa 


    return parcelStatus;

}
 


binder_status_t AIBinder_finalizeTransaction(AIBinder* binder, AParcel** out) {

    if (out == nullptr) {

        LOG(ERROR) << __func__ << ": requires non-null out parameter";

        return EX_NULL_POINTER;

    }



    // This object is the input to the transaction. This function takes ownership of it and deletes

    // it.

    AutoParcelDestroyer forOut(out, destroy_parcel);



    if (binder == nullptr || *out == nullptr) {

        LOG(ERROR) << __func__ << ": requires non-null parameters.";

        return EX_NULL_POINTER;

    }



    if ((*out)->getBinder() != binder) {

        LOG(ERROR) << __func__ << ": parcel is associated with binder object " << binder

                   << " but called with " << (*out)->getBinder();

        return EX_ILLEGAL_STATE;

    }



    if ((**out)->dataAvail() != 0) {

        LOG(ERROR) << __func__

                   << ": Only part of this transaction was read. There is remaining data left.";

        return EX_ILLEGAL_STATE;

    }



    return EX_NONE;

}

libs/binder/ndk/AParcel.cpp

+9 −0
Original line number Diff line number Diff line
@@ -25,6 +25,15 @@ using ::android::IBinder; 
using ::android::Parcel;

using ::android::sp;



void AParcel_delete(AParcel** parcel) {

    if (parcel == nullptr) {

        return;

    }



    delete *parcel;

    *parcel = nullptr;

}



binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder) {

    return (*parcel)->writeStrongBinder(binder->getBinder());

}

libs/binder/ndk/include_ndk/android/binder_ibinder.h

+8 −13
Original line number Diff line number Diff line
@@ -186,10 +186,9 @@ void* AIBinder_getUserData(AIBinder* binder); 
/**

 * A transaction is a series of calls to these functions which looks this

 * - call AIBinder_prepareTransaction

 * - fill out parcel with in parameters (lifetime of the 'in' variable)

 * - fill out the in parcel with parameters (lifetime of the 'in' variable)

 * - call AIBinder_transact

 * - fill out parcel with out parameters (lifetime of the 'out' variable)

 * - call AIBinder_finalizeTransaction

 * - read results from the out parcel (lifetime of the 'out' variable)

 */



/**
@@ -200,7 +199,10 @@ void* AIBinder_getUserData(AIBinder* binder); 
 * can be avoided. This AIBinder must be either built with a class or associated with a class before

 * using this API.

 *

 * This does not affect the ownership of binder.

 * This does not affect the ownership of binder. When this function succeeds, the in parcel's

 * ownership is passed to the caller. At this point, the parcel can be filled out and passed to

 * AIBinder_transact. Alternatively, if there is an error while filling out the parcel, it can be

 * deleted with AParcel_delete.

 */

binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in);
@@ -213,19 +215,12 @@ binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in); 
 * remote process has processed the transaction, and the out parcel will contain the output data

 * from transaction.

 *

 * This does not affect the ownership of binder.

 * This does not affect the ownership of binder. The out parcel's ownership is passed to the caller

 * and must be released with AParcel_delete when finished reading.

 */

binder_status_t AIBinder_transact(AIBinder* binder, transaction_code_t code, AParcel** in,

                                  AParcel** out, binder_flags_t flags);



/**

 * This takes ownership of the out parcel and automatically deletes it. Additional checks for

 * security or debugging maybe performed internally.

 *

 * This does not affect the ownership of binder.

 */

binder_status_t AIBinder_finalizeTransaction(AIBinder* binder, AParcel** out);



/*

 * This does not take any ownership of the input binder, but it can be used to retrieve it if

 * something else in some process still holds a reference to it.

libs/binder/ndk/include_ndk/android/binder_parcel.h

+5 −0
Original line number Diff line number Diff line
@@ -34,6 +34,11 @@ __BEGIN_DECLS 
struct AParcel;

typedef struct AParcel AParcel;



/**

 * Cleans up a parcel and sets it to nullptr.

 */

void AParcel_delete(AParcel** parcel);



/**

 * Writes an AIBinder to the next location in a non-null parcel. Can be null.

 */

libs/binder/ndk/test/iface.cpp

+2 −1
Original line number Diff line number Diff line
@@ -81,7 +81,8 @@ public: 
        int32_t out;

        CHECK(EX_NONE == AParcel_readInt32(parcelOut, &out));



        CHECK(EX_NONE == AIBinder_finalizeTransaction(mBinder, &parcelOut));

        AParcel_delete(&parcelOut);



        return out;

    }