YaraRuleBuilder allows empty string meta values (issue #58) #59
Conversation
| @@ -176,8 +176,8 @@ YaraRuleBuilder& YaraRuleBuilder::withComment(const std::string& comment, bool m | |||
| */ | |||
| YaraRuleBuilder& YaraRuleBuilder::withStringMeta(const std::string& key, const std::string& value) | |||
| { | |||
| if (key == std::string{} || value == std::string{}) | |||
| throw RuleBuilderError("Error: String-Meta key and value must be non-empty."); | |||
| if (key == std::string{}) | |||
There was a problem hiding this comment.
Tip: Here, we basically ask if key is empty. The more straightforward way would be to use if (key.empty()), which does exactly what we need and sounds better when read in English ("if key is empty"). Also, we will no longer need to create an empty string just to use it for comparison and then destroy it.
If you like the tip, feel free to commit it separately as it is not related to the present PR. I just wanted to point that out.
There was a problem hiding this comment.
Thank you for the tip, I totally agree. I will commit it in separate PR.
tests/cpp/builder_tests.cpp
Outdated
| @@ -141,6 +141,37 @@ RuleWithMetasWorks) { | |||
| )", yaraFile->getTextFormatted()); | |||
| } | |||
|
|
|||
| TEST_F(BuilderTests, | |||
| RuleWithEmptyStringMetasWorks) { | |||
There was a problem hiding this comment.
Consider naming the test RuleWithEmptyStringMetaValueWorks as we actually check that an empty value is permitted (an empty key is still forbidden).
There was a problem hiding this comment.
Thank you for your review, I have fixed the code as you suggested. The first of your tips will come as a separate PR.
YaraRuleBuilder allows now to create string meta with empty string as value.