BnFoo class cleanup.
BnFoo objects wrap an IFoo implementation, which
is stored in mImpl. But, BnFoo previously also
inherited from IFoo, and the implementation of those
methods just did a call-through on the wrapped
implementation.
What's wrong with this is that BnFoo is a transport
object, that we don't want to expose to clients
directly. This change fixes that by removing the
IFoo inheritance from BnFoo. A consequence of this
is also that, if A passed an IFoo to B, and B passes
this IFoo back to A, A will receive the exact same
IFoo object (its local service implementation).
Additionally, all BnFoo objects now inherit from
BnBase; BnBase is a common base class all BnFoo
objects share, so we can safely cast generic
IBinder objects back to BnFoo objects.
Test: mma, hidl_test, hidl_test_java
Bug: 33173166
Bug: 33492405
Change-Id: I6d6dd0db4810fa47af5a428547a1f7d2f0904d43
diff --git a/generateCpp.cpp b/generateCpp.cpp
index e282353..01aeaf9 100644
--- a/generateCpp.cpp
+++ b/generateCpp.cpp
@@ -375,9 +375,9 @@
out << "\n";
- out << "#include <hidl/HidlTransportSupport.h>\n";
out << "#include <hidl/Status.h>\n";
out << "#include <hwbinder/IBinder.h>\n";
+ out << "#include <hwbinder/Parcel.h>\n";
out << "\n";
@@ -399,43 +399,6 @@
return mRootScope->emitTypeDeclarations(out);
}
-status_t AST::generateStubMethod(Formatter &out,
- const Method *method) const {
- out << "inline ";
-
- method->generateCppSignature(out);
-
- const bool returnsValue = !method->results().empty();
- const TypedVar *elidedReturn = method->canElideCallback();
- out << " {\n";
- out.indent();
- out << "return mImpl->"
- << method->name()
- << "(";
- bool first = true;
- for (const auto &arg : method->args()) {
- if (!first) {
- out << ", ";
- }
- first = false;
- out << arg->name();
- }
- if (returnsValue && elidedReturn == nullptr) {
- if (!method->args().empty()) {
- out << ", ";
- }
-
- out << "_hidl_cb";
- }
- out << ");\n";
- out.unindent();
- out << "}";
-
- out << ";\n";
-
- return OK;
-}
-
status_t AST::generatePassthroughMethod(Formatter &out,
const Method *method) const {
method->generateCppSignature(out);
@@ -618,15 +581,23 @@
out << "struct "
<< "Bn"
- << baseName
- << " : public ::android::hardware::BnInterface<I"
- << baseName
- << ">, public ::android::hardware::HidlInstrumentor {\n";
+ << baseName;
+ if (iface->isIBase()) {
+ out << " : public ::android::hardware::BBinder";
+ out << ", public ::android::hardware::HidlInstrumentor {\n";
+ } else {
+ out << " : public ::android::hidl::base::V1_0::BnBase {\n";
+ }
out.indent();
out << "explicit Bn"
<< baseName
<< "(const ::android::sp<" << ifaceName << "> &_hidl_impl);"
+ << "\n";
+ out << "explicit Bn"
+ << baseName
+ << "(const ::android::sp<" << ifaceName << "> &_hidl_impl,"
+ << " const std::string& prefix);"
<< "\n\n";
out << "::android::status_t onTransact(\n";
out.indent();
@@ -639,15 +610,11 @@
out.unindent();
out.unindent();
- status_t err = generateMethods(out, [&](const Method *method, const Interface *) {
- return generateStubMethod(out, method);
- });
-
- if (err != OK) {
- return err;
- }
-
-
+ out << "::android::sp<" << ifaceName << "> getImpl() { return _hidl_mImpl; };\n";
+ out.unindent();
+ out << "private:\n";
+ out.indent();
+ out << "::android::sp<" << ifaceName << "> _hidl_mImpl;\n";
out.unindent();
out << "};\n\n";
@@ -773,7 +740,8 @@
Formatter out(file);
out << "#include <android/log.h>\n";
- out << "#include <cutils/trace.h>\n\n";
+ out << "#include <cutils/trace.h>\n";
+ out << "#include <hidl/HidlTransportSupport.h>\n\n";
if (isInterface) {
// This is a no-op for IServiceManager itself.
out << "#include <android/hidl/manager/1.0/IServiceManager.h>\n";
@@ -844,7 +812,7 @@
}
if (err == OK && isInterface) {
- err = generateStubSource(out, baseName);
+ err = generateStubSource(out, iface, baseName);
}
if (err == OK && isInterface) {
@@ -1148,7 +1116,9 @@
}
status_t AST::generateStubSource(
- Formatter &out, const std::string &baseName) const {
+ Formatter &out,
+ const Interface *iface,
+ const std::string &baseName) const {
const std::string klassName = "Bn" + baseName;
out << klassName
@@ -1159,20 +1129,47 @@
out.indent();
out.indent();
- out << ": BnInterface"
- << "<I"
- << baseName
- << ">(_hidl_impl),\n"
- << " ::android::hardware::HidlInstrumentor(\""
- << mPackage.string()
+ if (iface->isIBase()) {
+ out << ": ::android::hardware::HidlInstrumentor(\"";
+ } else {
+ out << ": ::android::hidl::base::V1_0::BnBase(_hidl_impl, \"";
+ }
+
+ out << mPackage.string()
<< "::I"
<< baseName
- << "\") {\n";
+ << "\") { \n";
+ out.indent();
+ out << "_hidl_mImpl = _hidl_impl;\n";
+ out.unindent();
out.unindent();
out.unindent();
out << "}\n\n";
+ if (iface->isIBase()) {
+ // BnBase has a constructor to initialize the HidlInstrumentor
+ // class properly.
+ out << klassName
+ << "::"
+ << klassName
+ << "(const ::android::sp<I" << baseName <<"> &_hidl_impl,"
+ << " const std::string &prefix)\n";
+
+ out.indent();
+ out.indent();
+
+ out << ": ::android::hardware::HidlInstrumentor(prefix) { \n";
+ out.indent();
+ out << "_hidl_mImpl = _hidl_impl;\n";
+ out.unindent();
+
+ out.unindent();
+ out.unindent();
+ out << "}\n\n";
+ }
+
+
out << "::android::status_t " << klassName << "::onTransact(\n";
out.indent();
@@ -1190,8 +1187,6 @@
out << "switch (_hidl_code) {\n";
out.indent();
- const Interface *iface = mRootScope->getInterface();
-
for (const auto &tuple : iface->allMethodsFromRoot()) {
const Method *method = tuple.method();
const Interface *superInterface = tuple.interface();
@@ -1217,9 +1212,7 @@
out << "default:\n{\n";
out.indent();
- out << "return ::android::hardware::BnInterface<I"
- << baseName
- << ">::onTransact(\n";
+ out << "return onTransact(\n";
out.indent();
out.indent();
@@ -1314,8 +1307,8 @@
out << elidedReturn->type().getCppResultType()
<< " _hidl_out_"
<< elidedReturn->name()
- << " = this->"
- << method->name()
+ << " = "
+ << "_hidl_mImpl->" << method->name()
<< "(";
bool first = true;
@@ -1368,7 +1361,7 @@
out << "bool _hidl_callbackCalled = false;\n\n";
}
- out << method->name() << "(";
+ out << "_hidl_mImpl->" << method->name() << "(";
bool first = true;
for (const auto &arg : method->args()) {