API: YR_COMPILER errors lead to undefined behavior

[Orycterope]

I found myself debugging an infinite loop in a rule that yara compiled. The reason was that the rule which emitted the buggy rule was invalid, compiler::add_rules returned an error, but because I'm doing a best effort, I ignored it, expecting yara would just throw it away, and kept adding other rules, called compiler::compile_rules, and used the obtain rules to do a scan.

The problem was that for the invalid rule, yara compiled it to bytecode that made no sense, like it completely gave up in the middle of the compilation, hence the infinite loop. But it was still executed by the scan.

Turns out reading the doc more closely, it says that

The yr_compiler_add_file(), yr_compiler_add_fd(), and yr_compiler_add_string() functions return the number of errors found in the source code. If the rules are correct they will return 0. If any of these functions return an error the compiler can't used anymore, neither for adding more rules nor getting the compiled rules.

This is not at all obvious in the api that the compiler provides, and right now it is too easy to end up with undefined behavior if you don't read the yara docs very closely.

I believe all the add_rules_* functions should take the compiler by move, like compiler::compile_rules does, instead of &mut self, and return a Result<Compiler, YaraError>, where Ok is the same compiler with the rule successfully added. That way it will be impossible for the user to use a Compiler after one of the yara functions returned an error.

I can do a PR for that, but it will be a breaking change. I wanted your opinion on it before doing it.

[ikrivosheev]

@Orycterope I think this is a good solution.

[Hugal31]

I think it is a good solution too.