[binder] Clean up parcel interfaces#27322
Conversation
TaWeiTu
added
the
release notes: no
TaWeiTu
force-pushed
the
cleanup-parcel-interface
branch
2 times, most recently
from
ce64eef to
fc46e6f
Compare
| AParcel* unused_output_parcel; | ||
| return AIBinder_transact(binder, static_cast<transaction_code_t>(tx_code), | ||
| &input_parcel_->parcel_, &output_parcel_->parcel_, | ||
| &input_parcel_->parcel_, &unused_output_parcel, |
There was a problem hiding this comment.
Oops, I think we leak memory here? We will need to call AParcel_delete to delete the output parcel
There was a problem hiding this comment.
How come we didn't notice this leak before?
There was a problem hiding this comment.
(I mean, it is my fault that in my first prototype I didn't do any memory management and leaves no TODO)
There was a problem hiding this comment.
You're right. I guess we weren't building Android code with Sanitizers before.
There was a problem hiding this comment.
Is it even possible to detect leak on Android?
Quick google search shows https://source.android.com/devices/tech/debug/hwasan but I'm not sure how effective it is
TaWeiTu
force-pushed
the
cleanup-parcel-interface
branch
2 times, most recently
from
41995b7 to
3417f17
Compare
Some changes: * OnTransactCb now takes a non-const ReadableParcel* so that testing codes no longer have to rely on mutable. * Remove GetReadableParcel() interface from binder since we only sent one-way transaction and the output (readable) parcel is never used. * Remove GetDataPosition() / SetDataPosition() interfaces since they are both unused. * Some changes that should've been made to grpc#27257 but was somehow missing...
TaWeiTu
force-pushed
the
cleanup-parcel-interface
branch
from
3417f17 to
8e1100d
Compare
|
@sifmelcara Can you take a look at this? Thanks! |
sifmelcara
left a comment
sifmelcara
left a comment
There was a problem hiding this comment.
Android CI failed although it looks unrelated to this change but we probably want to rerun in just to be safe
|
Android CI passed, so I will just enable auto-merge. |
Some changes: * OnTransactCb now takes a non-const ReadableParcel* so that testing codes no longer have to rely on mutable. * Remove GetReadableParcel() interface from binder since we only sent one-way transaction and the output (readable) parcel is never used. * Remove GetDataPosition() / SetDataPosition() interfaces since they are both unused. * Some changes that should've been made to grpc#27257 but was somehow missing...
2 participants
Some changes:
OnTransactCbnow takes a non-constReadableParcel*so that testingcodes no longer have to rely on
mutable.GetReadableParcel()interface from binder since we only sendone-way transaction and the output (readable) parcel is never used.
GetDataPosition()/SetDataPosition()interfaces since they areboth unused.