Merge changes from topic 'slimfast-ready'
* changes:
Move CompoundType definitions to headers.
Move all toString to be in users of HIDL libs.
diff --git a/AST.cpp b/AST.cpp
index cecdabd..2dfd7fa 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -116,7 +116,7 @@
for (const auto &subFQName : packageInterfaces) {
// Do not enforce restrictions on imports.
- AST *ast = mCoordinator->parse(subFQName, &mImportedASTs, false /* enforce */);
+ AST* ast = mCoordinator->parse(subFQName, &mImportedASTs, Coordinator::Enforce::NONE);
if (ast == nullptr) {
return false;
}
@@ -135,7 +135,7 @@
// assume it is an interface, and try to import it.
const FQName interfaceName = fqName.getTopLevelType();
// Do not enforce restrictions on imports.
- importAST = mCoordinator->parse(interfaceName, &mImportedASTs, false /* enforce */);
+ importAST = mCoordinator->parse(interfaceName, &mImportedASTs, Coordinator::Enforce::NONE);
if (importAST != nullptr) {
// cases like android.hardware.foo@1.0::IFoo.Internal
@@ -165,7 +165,7 @@
FQName typesFQName = fqName.getTypesForPackage();
// Do not enforce restrictions on imports.
- importAST = mCoordinator->parse(typesFQName, &mImportedASTs, false /* enforce */);
+ importAST = mCoordinator->parse(typesFQName, &mImportedASTs, Coordinator::Enforce::NONE);
if (importAST != nullptr) {
// Attempt to find Abc.Internal in types.
@@ -524,7 +524,7 @@
void AST::getAllImportedNames(std::set<FQName> *allImportNames) const {
for (const auto& name : mImportedNames) {
allImportNames->insert(name);
- AST *ast = mCoordinator->parse(name, nullptr /* imported */, false /* enforce */);
+ AST* ast = mCoordinator->parse(name, nullptr /* imported */, Coordinator::Enforce::NONE);
ast->getAllImportedNames(allImportNames);
}
}
diff --git a/Coordinator.cpp b/Coordinator.cpp
index 19cb381..6155fcf 100644
--- a/Coordinator.cpp
+++ b/Coordinator.cpp
@@ -63,7 +63,8 @@
}
}
-AST *Coordinator::parse(const FQName &fqName, std::set<AST *> *parsedASTs, bool enforce) const {
+AST* Coordinator::parse(const FQName& fqName, std::set<AST*>* parsedASTs,
+ Enforce enforcement) const {
CHECK(fqName.isFullyQualified());
auto it = mCache.find(fqName);
@@ -86,7 +87,7 @@
// Any interface file implicitly imports its package's types.hal.
FQName typesName = fqName.getTypesForPackage();
// Do not enforce on imports.
- typesAST = parse(typesName, parsedASTs, false /* enforce */);
+ typesAST = parse(typesName, parsedASTs, Enforce::NONE);
// fall through.
}
@@ -171,15 +172,13 @@
// parse fqName.
mCache[fqName] = ast;
- if (enforce) {
- // For each .hal file that hidl-gen parses, the whole package will be checked.
- err = enforceRestrictionsOnPackage(fqName);
- if (err != OK) {
- mCache[fqName] = nullptr;
- delete ast;
- ast = nullptr;
- return nullptr;
- }
+ // For each .hal file that hidl-gen parses, the whole package will be checked.
+ err = enforceRestrictionsOnPackage(fqName, enforcement);
+ if (err != OK) {
+ mCache[fqName] = nullptr;
+ delete ast;
+ ast = nullptr;
+ return nullptr;
}
return ast;
@@ -383,8 +382,11 @@
return packageRoot; // now converted to a path
}
+status_t Coordinator::enforceRestrictionsOnPackage(const FQName& fqName,
+ Enforce enforcement) const {
+ CHECK(enforcement == Enforce::FULL || enforcement == Enforce::NO_HASH ||
+ enforcement == Enforce::NONE);
-status_t Coordinator::enforceRestrictionsOnPackage(const FQName &fqName) const {
// need fqName to be something like android.hardware.foo@1.0.
// name and valueName is ignored.
if (fqName.package().empty() || fqName.version().empty()) {
@@ -392,6 +394,11 @@
<< ": package or version is missing.";
return BAD_VALUE;
}
+
+ if (enforcement == Enforce::NONE) {
+ return OK;
+ }
+
FQName package = fqName.getPackageAndVersion();
// look up cache.
if (mPackagesEnforced.find(package) != mPackagesEnforced.end()) {
@@ -406,9 +413,11 @@
return err;
}
- err = enforceHashes(package);
- if (err != OK) {
- return err;
+ if (enforcement != Enforce::NO_HASH) {
+ err = enforceHashes(package);
+ if (err != OK) {
+ return err;
+ }
}
// cache it so that it won't need to be enforced again.
diff --git a/Coordinator.h b/Coordinator.h
index c401995..2e761df 100644
--- a/Coordinator.h
+++ b/Coordinator.h
@@ -43,14 +43,20 @@
// adds path only if it doesn't exist
void addDefaultPackagePath(const std::string& root, const std::string& path);
+ enum class Enforce {
+ FULL, // default
+ NO_HASH, // only for use with -Lhash
+ NONE, // only for use during enforcement
+ };
+
// Attempts to parse the interface/types referred to by fqName.
// Parsing an interface also parses the associated package's types.hal
// file if it exists.
// If "parsedASTs" is non-NULL, successfully parsed ASTs are inserted
// into the set.
// If !enforce, enforceRestrictionsOnPackage won't be run.
- AST *parse(const FQName &fqName, std::set<AST *> *parsedASTs = nullptr,
- bool enforce = true) const;
+ AST* parse(const FQName& fqName, std::set<AST*>* parsedASTs = nullptr,
+ Enforce enforcement = Enforce::FULL) const;
// Given package-root paths of ["hardware/interfaces",
// "vendor/<something>/interfaces"], package roots of
@@ -94,7 +100,8 @@
// - minor version upgrades
// "packages" contains names like "android.hardware.nfc@1.1".
// - hashing restrictions
- status_t enforceRestrictionsOnPackage(const FQName &fqName) const;
+ status_t enforceRestrictionsOnPackage(const FQName& fqName,
+ Enforce enforcement = Enforce::FULL) const;
static bool MakeParentHierarchy(const std::string &path);
diff --git a/PointerType.cpp b/PointerType.cpp
index 53dda83..4947e9e 100644
--- a/PointerType.cpp
+++ b/PointerType.cpp
@@ -42,11 +42,12 @@
void PointerType::emitReaderWriter(
Formatter& out,
- const std::string& /* name */,
+ const std::string& name,
const std::string& /* parcelObj */,
bool /* parcelObjIsPointer */,
bool /* isReader */,
ErrorMode /* mode */) const {
+ out << "(void)" << name << ";\n";
out << "LOG_ALWAYS_FATAL(\"Pointer is only supported in passthrough mode\");\n";
}
diff --git a/generateCpp.cpp b/generateCpp.cpp
index 9f07fdc..04033a7 100644
--- a/generateCpp.cpp
+++ b/generateCpp.cpp
@@ -1160,7 +1160,8 @@
out.sIf(nonNull + " == nullptr", [&] {
out << "return ::android::hardware::Status::fromExceptionCode(\n";
out.indent(2, [&] {
- out << "::android::hardware::Status::EX_ILLEGAL_ARGUMENT);\n";
+ out << "::android::hardware::Status::EX_ILLEGAL_ARGUMENT,\n"
+ << "\"Null synchronous callback passed.\");\n";
});
}).endl().endl();
}
@@ -1984,7 +1985,8 @@
out << "return ::android::hardware::Status::fromExceptionCode(\n";
out.indent();
out.indent();
- out << "::android::hardware::Status::EX_TRANSACTION_FAILED);\n";
+ out << "::android::hardware::Status::EX_TRANSACTION_FAILED,\n"
+ << "\"Passthrough oneway function queue exceeds maximum size.\");\n";
out.unindent();
out.unindent();
out.unindent();
diff --git a/hidl-gen_l.ll b/hidl-gen_l.ll
index 5dd3c85..5945df9 100644
--- a/hidl-gen_l.ll
+++ b/hidl-gen_l.ll
@@ -23,9 +23,9 @@
COMPONENT {L}({L}|{D})*
DOT [.]
-PATH {COMPONENT}({DOT}{COMPONENT})*
AT [@]
VERSION {AT}{D}+{DOT}{D}+
+FQNAME ({COMPONENT}|{VERSION})(({DOT}|":"+){COMPONENT}|{VERSION})*
%{
@@ -157,15 +157,8 @@
"?" { return('?'); }
"@" { return('@'); }
-{PATH}{VERSION}"::"{PATH} { yylval->str = strdup(yytext); return token::FQNAME; }
-{VERSION}"::"{PATH} { yylval->str = strdup(yytext); return token::FQNAME; }
-{PATH}{VERSION} { yylval->str = strdup(yytext); return token::FQNAME; }
-{COMPONENT}({DOT}{COMPONENT})+ { yylval->str = strdup(yytext); return token::FQNAME; }
{COMPONENT} { yylval->str = strdup(yytext); return token::IDENTIFIER; }
-
-{PATH}{VERSION}"::"{PATH}":"{COMPONENT} { yylval->str = strdup(yytext); return token::FQNAME; }
-{VERSION}"::"{PATH}":"{COMPONENT} { yylval->str = strdup(yytext); return token::FQNAME; }
-{PATH}":"{COMPONENT} { yylval->str = strdup(yytext); return token::FQNAME; }
+{FQNAME} { yylval->str = strdup(yytext); return token::FQNAME; }
0[xX]{H}+{IS}? { yylval->str = strdup(yytext); return token::INTEGER; }
0{D}+{IS}? { yylval->str = strdup(yytext); return token::INTEGER; }
diff --git a/hidl-gen_y.yy b/hidl-gen_y.yy
index 56bb605..35f7405 100644
--- a/hidl-gen_y.yy
+++ b/hidl-gen_y.yy
@@ -904,7 +904,9 @@
$$ = $2;
if ($$ != NULL && !$$->isValidEnumStorageType()) {
- std::cerr << "ERROR: Invalid enum storage type specified. at "
+ std::cerr << "ERROR: Invalid enum storage type ("
+ << $2->typeName()
+ << ") specified. at "
<< @2 << "\n";
YYERROR;
diff --git a/main.cpp b/main.cpp
index 69da62d..3c29a09 100644
--- a/main.cpp
+++ b/main.cpp
@@ -643,6 +643,11 @@
return true;
}
+bool isHidlTransportPackage(const FQName &package) {
+ return package == gIBasePackageFqName ||
+ package == gIManagerPackageFqName;
+}
+
static void generateAndroidBpGenSection(
Formatter &out,
const FQName &packageFQName,
@@ -685,9 +690,66 @@
out << "}\n\n";
}
-bool isHidlTransportPackage(const FQName &package) {
- return package == gIBasePackageFqName ||
- package == gIManagerPackageFqName;
+static void generateAndroidBpLibSection(
+ Formatter &out,
+ bool generateVendor,
+ const std::string &libraryName,
+ const std::string &genSourceName,
+ const std::string &genHeaderName,
+ const std::set<FQName> &importedPackagesHierarchy) {
+
+ // C++ library definition
+ out << "cc_library_shared {\n";
+ out.indent();
+ out << "name: \"" << libraryName << (generateVendor ? "_vendor" : "") << "\",\n"
+ << "defaults: [\"hidl-module-defaults\"],\n"
+ << "generated_sources: [\"" << genSourceName << "\"],\n"
+ << "generated_headers: [\"" << genHeaderName << "\"],\n"
+ << "export_generated_headers: [\"" << genHeaderName << "\"],\n";
+
+ if (generateVendor) {
+ out << "vendor: true,\n";
+ } else {
+ out << "vendor_available: true,\n";
+ }
+ out << "shared_libs: [\n";
+
+ out.indent();
+ out << "\"libhidlbase\",\n"
+ << "\"libhidltransport\",\n"
+ << "\"libhwbinder\",\n"
+ << "\"liblog\",\n"
+ << "\"libutils\",\n"
+ << "\"libcutils\",\n";
+ for (const auto &importedPackage : importedPackagesHierarchy) {
+ if (isHidlTransportPackage(importedPackage)) {
+ continue;
+ }
+
+ out << "\"" << makeLibraryName(importedPackage) << "\",\n";
+ }
+ out.unindent();
+
+ out << "],\n";
+
+ out << "export_shared_lib_headers: [\n";
+ out.indent();
+ out << "\"libhidlbase\",\n"
+ << "\"libhidltransport\",\n"
+ << "\"libhwbinder\",\n"
+ << "\"libutils\",\n";
+ for (const auto &importedPackage : importedPackagesHierarchy) {
+ if (isHidlTransportPackage(importedPackage)) {
+ continue;
+ }
+
+ out << "\"" << makeLibraryName(importedPackage) << "\",\n";
+ }
+ out.unindent();
+ out << "],\n";
+ out.unindent();
+
+ out << "}\n";
}
static status_t generateAndroidBpForPackage(
@@ -813,64 +875,40 @@
if (isHidlTransportPackage(packageFQName)) {
out << "// " << packageFQName.string() << " is exported from libhidltransport\n";
} else {
- // C++ library definition
- out << "cc_library_shared {\n";
- out.indent();
- out << "name: \"" << libraryName << "\",\n"
- << "defaults: [\"hidl-module-defaults\"],\n"
- << "generated_sources: [\"" << genSourceName << "\"],\n"
- << "generated_headers: [\"" << genHeaderName << "\"],\n"
- << "export_generated_headers: [\"" << genHeaderName << "\"],\n";
+ generateAndroidBpLibSection(
+ out,
+ false /* generateVendor */,
+ libraryName,
+ genSourceName,
+ genHeaderName,
+ importedPackagesHierarchy);
- // TODO(b/35813011): make always vendor_available
- // Explicitly mark libraries vendor until BOARD_VNDK_VERSION can
- // be enabled.
- if (packageFQName.inPackage("android.hidl") ||
+ // TODO(b/35813011): make all libraries vendor_available
+ // Explicitly create '_vendor' copies of libraries so that
+ // vendor code can link against the extensions. When this is
+ // used, framework code should link against vendor.awesome.foo@1.0
+ // and code on the vendor image should link against
+ // vendor.awesome.foo@1.0_vendor. For libraries with the below extensions,
+ // they will be available even on the generic system image.
+ // Because of this, they should always be referenced without the
+ // '_vendor' name suffix.
+ if (!(packageFQName.inPackage("android.hidl") ||
packageFQName.inPackage("android.system") ||
packageFQName.inPackage("android.frameworks") ||
- packageFQName.inPackage("android.hardware")) {
- out << "vendor_available: true,\n";
- } else {
- out << "vendor: true,\n";
+ packageFQName.inPackage("android.hardware"))) {
+
+ // Note, not using cc_defaults here since it's already not used and
+ // because generating this libraries will be removed when the VNDK
+ // is enabled (done by the build system itself).
+ out.endl();
+ generateAndroidBpLibSection(
+ out,
+ true /* generateVendor */,
+ libraryName,
+ genSourceName,
+ genHeaderName,
+ importedPackagesHierarchy);
}
- out << "shared_libs: [\n";
-
- out.indent();
- out << "\"libhidlbase\",\n"
- << "\"libhidltransport\",\n"
- << "\"libhwbinder\",\n"
- << "\"liblog\",\n"
- << "\"libutils\",\n"
- << "\"libcutils\",\n";
- for (const auto &importedPackage : importedPackagesHierarchy) {
- if (isHidlTransportPackage(importedPackage)) {
- continue;
- }
-
- out << "\"" << makeLibraryName(importedPackage) << "\",\n";
- }
- out.unindent();
-
- out << "],\n";
-
- out << "export_shared_lib_headers: [\n";
- out.indent();
- out << "\"libhidlbase\",\n"
- << "\"libhidltransport\",\n"
- << "\"libhwbinder\",\n"
- << "\"libutils\",\n";
- for (const auto &importedPackage : importedPackagesHierarchy) {
- if (isHidlTransportPackage(importedPackage)) {
- continue;
- }
-
- out << "\"" << makeLibraryName(importedPackage) << "\",\n";
- }
- out.unindent();
- out << "],\n";
- out.unindent();
-
- out << "}\n";
}
return OK;
@@ -1109,7 +1147,8 @@
}
for (const auto ¤tFqName : packageInterfaces) {
- AST *ast = coordinator->parse(currentFqName);
+ AST* ast = coordinator->parse(currentFqName, {} /* parsed */,
+ Coordinator::Enforce::NO_HASH /* enforcement */);
if (ast == NULL) {
fprintf(stderr,
@@ -1329,6 +1368,12 @@
argc -= optind;
argv += optind;
+ if (argc == 0) {
+ fprintf(stderr, "ERROR: no fqname specified.\n");
+ usage(me);
+ exit(1);
+ }
+
if (rootPath.empty()) {
const char *ANDROID_BUILD_TOP = getenv("ANDROID_BUILD_TOP");
diff --git a/test/hash_test/Android.bp b/test/hash_test/Android.bp
index aa58da9..89c403b 100644
--- a/test/hash_test/Android.bp
+++ b/test/hash_test/Android.bp
@@ -13,6 +13,11 @@
" -r test.hash:system/tools/hidl/test/hash_test/bad" +
" test.hash.hash@1.0 2> /dev/null)" +
"&&" +
+ "$(location hidl-gen) -L hash " +
+ " -r android.hidl:system/libhidl/transport" +
+ " -r test.hash:system/tools/hidl/test/hash_test/bad" +
+ " test.hash.hash@1.0 > /dev/null" +
+ "&&" +
"echo 'int main(){return 0;}' > $(genDir)/TODO_b_37575883.cpp",
out: ["TODO_b_37575883.cpp"],
diff --git a/test/hidl_test_client.cpp b/test/hidl_test_client.cpp
index 40d63f9..75fc633 100644
--- a/test/hidl_test_client.cpp
+++ b/test/hidl_test_client.cpp
@@ -1190,6 +1190,13 @@
}));
}
+TEST_F(HidlTest, FooNullSynchronousCallbackTest) {
+ Return<void> ret = foo->echoNullInterface(nullptr, nullptr /* synchronous callback */);
+
+ EXPECT_FAIL(ret);
+ EXPECT_TRUE(ret.description().find("Null synchronous callback passed") != std::string::npos);
+}
+
TEST_F(HidlTest, FooNullCallbackTest) {
EXPECT_OK(foo->echoNullInterface(nullptr,
[](const auto receivedNull, const auto &intf) {
diff --git a/test/vendor/1.0/Android.bp b/test/vendor/1.0/Android.bp
index ada44f7..d76a955 100644
--- a/test/vendor/1.0/Android.bp
+++ b/test/vendor/1.0/Android.bp
@@ -45,6 +45,31 @@
generated_sources: ["tests.vendor@1.0_genc++"],
generated_headers: ["tests.vendor@1.0_genc++_headers"],
export_generated_headers: ["tests.vendor@1.0_genc++_headers"],
+ vendor_available: true,
+ shared_libs: [
+ "libhidlbase",
+ "libhidltransport",
+ "libhwbinder",
+ "liblog",
+ "libutils",
+ "libcutils",
+ "android.hardware.tests.baz@1.0",
+ ],
+ export_shared_lib_headers: [
+ "libhidlbase",
+ "libhidltransport",
+ "libhwbinder",
+ "libutils",
+ "android.hardware.tests.baz@1.0",
+ ],
+}
+
+cc_library_shared {
+ name: "tests.vendor@1.0_vendor",
+ defaults: ["hidl-module-defaults"],
+ generated_sources: ["tests.vendor@1.0_genc++"],
+ generated_headers: ["tests.vendor@1.0_genc++_headers"],
+ export_generated_headers: ["tests.vendor@1.0_genc++_headers"],
vendor: true,
shared_libs: [
"libhidlbase",