AFLplusplus
Remove `observer_stdout` and `observer_stderr` from `ObserverTuple`
This one. https://github.com/AFLplusplus/LibAFL/pull/856
Why would a observer tuple have these specific functions ?
(Continuing the discussion from #2138)
StdOutObservers etc. are fairly closely coupled to the specific Executor they work with, since they need close interaction. @tokatoka's suggestion in #2138:
When instantiate this command executor, give the Handle or name of the exit code observer When you want to check the exit signal, search it by executor.observers.match_name
I don't like this particularly, to be honest. It relies on the user passing around magic variables (which includes all the possible problems like non-uniqueness, typos, etc.), and it is dependent on them knowing that they do, otherwise they will likely be presented by an error message that is fairly hard to decode.
In a perfect world, I'd argue that creation of the Executor-dependant Observers should be handled by the Executor itself. This way, it can retain a reference to use directly, without relying on magic variables, and creation is only possible for Executors that actually implement the functionality. This also makes it easier to use.
Yes, this would introduce tight coupling where there does not seem to be right now, but it is there under the hood regardless, and inherently necessary, because of the logical relationship between Executor-specific Observers and their Executors. It would just make the coupling transparent to the user and thus make it easier to understand what is supported.
Maybe a signature like this on CommandExecutor:
fn create_stdout_observer(&mut self, &str name) -> StdOutObserver;
(Disclaimer: I haven't checked if the borrow-checker would allow such constructs.)
I totally understand your point. indeed these observers are strictly tied to the executors. but I don't really think we should create an observer from the executor.
I'll rewrite the creation of the command executor using the builder pattern. so hopefully that'll make thigs better..