Update forkserver.rs #3138
Open
Update forkserver.rs #3138
+83
−8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're adding these comments in strategic spots where they'll be most useful - mainly at the public API boundaries like struct and function definitions. The style is concise but informative, focusing on explaining what each component does and how to use it properly.
|
can you run fmt and clippy? then it's good |
domenukk
reviewed
Apr 7, 2025
| //! the program from scratch. | ||
| //! | ||
| //! Key features: | ||
| //! - Support for LibAFL (libafl_cc/libafl_cxx) instrumented binaries |
There was a problem hiding this comment.
... if they use the [whatever its name was] feature
domenukk
reviewed
Apr 7, 2025
| /// * `Result<(), Error>` - Always returns `Err` with a descriptive error message | ||
| /// that explains the failure and suggests possible solutions | ||
| /// | ||
| /// # Error Codes |
There was a problem hiding this comment.
This does not return error codes but instead a string.
(Should these errors really return Error::unknown, though? Probably that should be more precise)
There was a problem hiding this comment.
We could:
- Error::configuration() for map size issues
- Error::compilation() for code compilation-related errors
- Error::system() for system call failures
- Error::version() for version incompatibility issues
There was a problem hiding this comment.
domenukk
reviewed
Apr 7, 2025
libafl/src/executors/forkserver.rs
Outdated
| /// | ||
| /// Shared memory feature is also available, but you have to set things up in your code. | ||
| /// Please refer to AFL++'s docs. <https://github.com/AFLplusplus/AFLplusplus/blob/stable/instrumentation/README.persistent_mode.md> | ||
| /// # Type Parameters |
There was a problem hiding this comment.
I think if we start documenting type parameters, they will eventually be refactored and forgotten.
Instead we should document common letters for things in a common place, and drop it from this function.
domenukk
reviewed
Apr 7, 2025
libafl/src/executors/forkserver.rs
Outdated
| @@ -769,7 +840,17 @@ where | |||
| } | |||
| } | |||
|
|
|||
| /// The builder for `ForkserverExecutor` | |||
| /// Builder for `ForkserverExecutor` with a fluent interface for configuration | |||
There was a problem hiding this comment.
you could write
[`ForkserverExecutor`]with brackets
domenukk
reviewed
Apr 7, 2025
libafl/src/executors/forkserver.rs
Outdated
| /// The builder for `ForkserverExecutor` | ||
| /// Builder for `ForkserverExecutor` with a fluent interface for configuration | ||
| /// | ||
| /// Provides methods to customize all aspects of forkserver execution: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We're adding these comments in strategic spots where they'll be most useful - mainly at the public API boundaries like struct and function definitions. The style is concise but informative, focusing on explaining what each component does and how to use it properly.
I'm targeting places where a good comment saves users from having to dig through implementation details. The triple-slash style (///) ensures these show up in the generated documentation, making the library more approachable for new users.
The goal is to strike a balance - enough detail to be helpful without becoming overwhelming. Comments that focus on "why" and "how" tend to be more valuable than those that just restate what the code is doing.