Packet: Refactor packet generation executable
* Split packet parsing and header generation into two separate functions
such that the parsing results can be used separately later
* Use Google style for functional arguments:
- const reference for input
- pointers for output
Bug: 143374372
Test: make, gd and packet unit tests
Change-Id: Id8ff4e805dc8e1b21dde50bdafdd8cf9bba36998
diff --git a/gd/packet/parser/main.cc b/gd/packet/parser/main.cc
index 8c6e4e1..a53d71d 100644
--- a/gd/packet/parser/main.cc
+++ b/gd/packet/parser/main.cc
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#include <errno.h>
#include <unistd.h>
+#include <cerrno>
#include <cstdio>
#include <filesystem>
#include <fstream>
@@ -37,21 +37,21 @@
namespace {
-void parse_namespace(std::string root_namespace, std::filesystem::path input_file_relative_path,
- std::vector<std::string>& token) {
+void parse_namespace(const std::string& root_namespace, const std::filesystem::path& input_file_relative_path,
+ std::vector<std::string>* token) {
std::filesystem::path gen_namespace = root_namespace / input_file_relative_path;
std::string gen_namespace_str = gen_namespace;
std::regex path_tokenizer("/");
auto it = std::sregex_token_iterator(gen_namespace_str.cbegin(), gen_namespace_str.cend(), path_tokenizer, -1);
- std::sregex_token_iterator it_end;
+ std::sregex_token_iterator it_end = {};
for (; it != it_end; ++it) {
- token.push_back(it->str());
+ token->push_back(it->str());
}
}
void generate_namespace_open(const std::vector<std::string>& token, std::ostream& output) {
- for (auto it = token.begin(); it != token.end(); ++it) {
- output << "namespace " << *it << " {" << std::endl;
+ for (const auto& ns : token) {
+ output << "namespace " << ns << " {" << std::endl;
}
}
@@ -61,17 +61,7 @@
}
}
-bool parse_one_file(std::filesystem::path input_file, std::filesystem::path include_dir, std::filesystem::path out_dir,
- std::string root_namespace) {
- auto gen_relative_path = input_file.lexically_relative(include_dir).parent_path();
-
- auto input_filename = input_file.filename().string().substr(0, input_file.filename().string().find(".pdl"));
- auto gen_path = out_dir / gen_relative_path;
-
- std::filesystem::create_directories(gen_path);
-
- auto gen_file = gen_path / (input_filename + ".h");
-
+bool parse_declarations_one_file(const std::filesystem::path& input_file, Declarations* declarations) {
void* scanner;
yylex_init(&scanner);
@@ -83,6 +73,43 @@
yyset_in(in_file, scanner);
+ int ret = yy::parser(scanner, declarations).parse();
+ if (ret != 0) {
+ std::cerr << "yylex parsing failed: returned " << ret << std::endl;
+ return false;
+ }
+
+ yylex_destroy(scanner);
+
+ fclose(in_file);
+
+ // Set endianess before returning
+ for (auto& s : declarations->type_defs_queue_) {
+ if (s.second->GetDefinitionType() == TypeDef::Type::STRUCT) {
+ auto* struct_def = dynamic_cast<StructDef*>(s.second);
+ struct_def->SetEndianness(declarations->is_little_endian);
+ }
+ }
+
+ for (auto& packet_def : declarations->packet_defs_queue_) {
+ packet_def.second.SetEndianness(declarations->is_little_endian);
+ }
+
+ return true;
+}
+
+bool generate_cpp_headers_one_file(const Declarations& decls, const std::filesystem::path& input_file,
+ const std::filesystem::path& include_dir, const std::filesystem::path& out_dir,
+ const std::string& root_namespace) {
+ auto gen_relative_path = input_file.lexically_relative(include_dir).parent_path();
+
+ auto input_filename = input_file.filename().string().substr(0, input_file.filename().string().find(".pdl"));
+ auto gen_path = out_dir / gen_relative_path;
+
+ std::filesystem::create_directories(gen_path);
+
+ auto gen_file = gen_path / (input_filename + ".h");
+
std::ofstream out_file;
out_file.open(gen_file);
if (!out_file.is_open()) {
@@ -90,16 +117,6 @@
return false;
}
- Declarations decls;
- int ret = yy::parser(scanner, &decls).parse();
-
- yylex_destroy(scanner);
-
- if (ret != 0) {
- std::cerr << "yylex parsing failed: returned " << ret << std::endl;
- return false;
- }
-
out_file << "\n\n";
out_file << "#pragma once\n";
out_file << "\n\n";
@@ -127,7 +144,7 @@
out_file << "\n\n";
std::vector<std::string> namespace_list;
- parse_namespace(root_namespace, gen_relative_path, namespace_list);
+ parse_namespace(root_namespace, gen_relative_path, &namespace_list);
generate_namespace_open(namespace_list, out_file);
out_file << "\n\n";
@@ -152,36 +169,40 @@
for (const auto& e : decls.type_defs_queue_) {
if (e.second->GetDefinitionType() == TypeDef::Type::ENUM) {
- EnumGen gen(*(EnumDef*)e.second);
+ const auto* enum_def = dynamic_cast<const EnumDef*>(e.second);
+ EnumGen gen(*enum_def);
gen.GenDefinition(out_file);
out_file << "\n\n";
}
}
for (const auto& e : decls.type_defs_queue_) {
if (e.second->GetDefinitionType() == TypeDef::Type::ENUM) {
- EnumGen gen(*(EnumDef*)e.second);
+ const auto* enum_def = dynamic_cast<const EnumDef*>(e.second);
+ EnumGen gen(*enum_def);
gen.GenLogging(out_file);
out_file << "\n\n";
}
}
for (const auto& ch : decls.type_defs_queue_) {
if (ch.second->GetDefinitionType() == TypeDef::Type::CHECKSUM) {
- ((ChecksumDef*)ch.second)->GenChecksumCheck(out_file);
+ const auto* checksum_def = dynamic_cast<const ChecksumDef*>(ch.second);
+ checksum_def->GenChecksumCheck(out_file);
}
}
out_file << "\n/* Done ChecksumChecks */\n";
for (const auto& c : decls.type_defs_queue_) {
if (c.second->GetDefinitionType() == TypeDef::Type::CUSTOM && c.second->size_ == -1 /* Variable Size */) {
- ((CustomFieldDef*)c.second)->GenCustomFieldCheck(out_file, decls.is_little_endian);
+ const auto* custom_field_def = dynamic_cast<const CustomFieldDef*>(c.second);
+ custom_field_def->GenCustomFieldCheck(out_file, decls.is_little_endian);
}
}
out_file << "\n";
for (auto& s : decls.type_defs_queue_) {
if (s.second->GetDefinitionType() == TypeDef::Type::STRUCT) {
- ((StructDef*)s.second)->SetEndianness(decls.is_little_endian);
- ((StructDef*)s.second)->GenDefinition(out_file);
+ const auto* struct_def = dynamic_cast<const StructDef*>(s.second);
+ struct_def->GenDefinition(out_file);
out_file << "\n";
}
}
@@ -192,21 +213,19 @@
out_file << "\n\n";
}
- for (size_t i = 0; i < decls.packet_defs_queue_.size(); i++) {
- decls.packet_defs_queue_[i].second.SetEndianness(decls.is_little_endian);
- decls.packet_defs_queue_[i].second.GenParserDefinition(out_file);
+ for (const auto& packet_def : decls.packet_defs_queue_) {
+ packet_def.second.GenParserDefinition(out_file);
out_file << "\n\n";
}
- for (const auto p : decls.packet_defs_queue_) {
- p.second.GenBuilderDefinition(out_file);
+ for (const auto& packet_def : decls.packet_defs_queue_) {
+ packet_def.second.GenBuilderDefinition(out_file);
out_file << "\n\n";
}
generate_namespace_close(namespace_list, out_file);
out_file.close();
- fclose(in_file);
return true;
}
@@ -245,10 +264,15 @@
}
while (!input_files.empty()) {
- if (!parse_one_file(input_files.front(), include_dir, out_dir, root_namespace)) {
- std::cerr << "Didn't parse " << input_files.front() << " correctly" << std::endl;
+ Declarations declarations;
+ if (!parse_declarations_one_file(input_files.front(), &declarations)) {
+ std::cerr << "Cannot parse " << input_files.front() << " correctly" << std::endl;
return 2;
}
+ if (!generate_cpp_headers_one_file(declarations, input_files.front(), include_dir, out_dir, root_namespace)) {
+ std::cerr << "Didn't generate cpp headers for " << input_files.front() << std::endl;
+ return 3;
+ }
input_files.pop();
}
diff --git a/gd/packet/parser/struct_parser_generator.cc b/gd/packet/parser/struct_parser_generator.cc
index c5ccb88..352219e 100644
--- a/gd/packet/parser/struct_parser_generator.cc
+++ b/gd/packet/parser/struct_parser_generator.cc
@@ -16,11 +16,12 @@
#include "struct_parser_generator.h"
-StructParserGenerator::StructParserGenerator(Declarations& decls) {
+StructParserGenerator::StructParserGenerator(const Declarations& decls) {
is_little_endian = decls.is_little_endian;
- for (auto& s : decls.type_defs_queue_) {
+ for (const auto& s : decls.type_defs_queue_) {
if (s.second->GetDefinitionType() == TypeDef::Type::STRUCT) {
- variable_struct_fields_.push_back((StructDef*)s.second);
+ const auto* struct_def = dynamic_cast<const StructDef*>(s.second);
+ variable_struct_fields_.emplace_back(struct_def);
}
}
for (const auto& node : variable_struct_fields_) {
@@ -36,10 +37,10 @@
void StructParserGenerator::explore_children(const TreeNode& node, std::ostream& s) const {
auto field = node.packet_field_;
- if (node.children_.size() > 0) {
+ if (!node.children_.empty()) {
s << "bool " << field->GetName() << "_child_found = false; /* Greedy match */";
}
- for (const auto child : node.children_) {
+ for (const auto& child : node.children_) {
s << "if (!" << field->GetName() << "_child_found && ";
s << child->struct_def_->name_ << "::IsInstance(*" << field->GetName() << "_value.get())) {";
s << field->GetName() << "_child_found = true;";
@@ -65,32 +66,33 @@
void StructParserGenerator::Generate(std::ostream& s) const {
for (const auto& node : variable_struct_fields_) {
- if (node.children_.size() > 0) {
- auto field = node.packet_field_;
- s << "inline std::unique_ptr<" << node.struct_def_->name_ << "> Parse" << node.struct_def_->name_;
- if (is_little_endian) {
- s << "(Iterator<kLittleEndian> to_bound) {";
- } else {
- s << "(Iterator<!kLittleEndian> to_bound) {";
- }
- s << field->GetDataType() << " " << field->GetName() << "_value = ";
- s << "std::make_unique<" << node.struct_def_->name_ << ">();";
-
- s << "auto " << field->GetName() << "_it = to_bound;";
- s << "auto optional_it = ";
- s << node.struct_def_->name_ << "::Parse( " << field->GetName() << "_value.get(), ";
- s << field->GetName() << "_it";
- if (node.struct_def_->parent_ != nullptr) {
- s << ", true);";
- } else {
- s << ");";
- }
- s << "if (optional_it) {";
- s << field->GetName() << "_it = *optional_it;";
- s << "} else { return nullptr; }";
-
- explore_children(node, s);
- s << "return " << field->GetName() << "_value; }";
+ if (node.children_.empty()) {
+ continue;
}
+ auto field = node.packet_field_;
+ s << "inline std::unique_ptr<" << node.struct_def_->name_ << "> Parse" << node.struct_def_->name_;
+ if (is_little_endian) {
+ s << "(Iterator<kLittleEndian> to_bound) {";
+ } else {
+ s << "(Iterator<!kLittleEndian> to_bound) {";
+ }
+ s << field->GetDataType() << " " << field->GetName() << "_value = ";
+ s << "std::make_unique<" << node.struct_def_->name_ << ">();";
+
+ s << "auto " << field->GetName() << "_it = to_bound;";
+ s << "auto optional_it = ";
+ s << node.struct_def_->name_ << "::Parse( " << field->GetName() << "_value.get(), ";
+ s << field->GetName() << "_it";
+ if (node.struct_def_->parent_ != nullptr) {
+ s << ", true);";
+ } else {
+ s << ");";
+ }
+ s << "if (optional_it) {";
+ s << field->GetName() << "_it = *optional_it;";
+ s << "} else { return nullptr; }";
+
+ explore_children(node, s);
+ s << "return " << field->GetName() << "_value; }";
}
}
diff --git a/gd/packet/parser/struct_parser_generator.h b/gd/packet/parser/struct_parser_generator.h
index 2c1ff6d..0315460 100644
--- a/gd/packet/parser/struct_parser_generator.h
+++ b/gd/packet/parser/struct_parser_generator.h
@@ -24,14 +24,14 @@
class StructParserGenerator {
public:
- StructParserGenerator(Declarations& declarations);
+ explicit StructParserGenerator(const Declarations& declarations);
void Generate(std::ostream& s) const;
private:
class TreeNode {
public:
- TreeNode(const StructDef* s)
+ explicit TreeNode(const StructDef* s)
: struct_def_(s), packet_field_(s->GetNewField(s->name_ + "_parse", ParseLocation())) {}
const StructDef* struct_def_;
const PacketField* packet_field_;