Don't check hash while generating hash.
A common usecase of -Lhash is to add a new hash to
a current.txt file. However, doing so requires removing
old hashes which then must be re-added for correctness.
This is because hashes of released interfaces should never be
removed.
Here I've made it so that -Lhash will return a hash for
valid interfaces even if the hash fails. I've also added
this to hidl_hash_test.
Test: hidl_hash_test
Bug: 62858322
Change-Id: I840086ae59656c73c7109b20c142da27b47f80f8
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/main.cpp b/main.cpp
index b7d5a96..3c29a09 100644
--- a/main.cpp
+++ b/main.cpp
@@ -1147,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,
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"],