Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Randomized generation and misc. fixes #499

Merged
merged 5 commits into from
Oct 12, 2024

Conversation

silentbicycle
Copy link
Collaborator

Instead of always using the first character in a label set, pseudorandomly pick one using rand() if a non-zero seed is provided. This adds an extra argument to fsm_generate_matches, for the seed.

Also:

  • Remove a stale comment.
  • Default TRACK_TIMES to 0 when BUILD_FOR_FUZZER is set, because it fills up the fuzzer logs with lots of uninteresting messages about things taking longer when subjected to weird random inputs.
  • Only run fsm_trim's integrity checks when EXPENSIVE_CHECKS is set.

There isn't a flag to set the seed in fsm or re yet.

Update callers, add a 0 seed argument to default to the
existing behavior.

Also, no end states means nothing to do, so exit right away.
It fills up the fuzzer logs with lots of uninteresting output.
src/libfsm/gen.c Outdated
(void)seed;
if (randomized) {
srand(seed);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about calling srand() in library code, because of its state being side-effect to the caller's own uses. I don't want to include our own prng. What do you think of moving the srand() invocation to the caller?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Should the interface just have bool randomized (or int, since nothing else in that header uses bool), note that it uses rand specifically, and encourage the caller to seed with srand? It doesn't need to be a good PRNG, just something that has more variation than always choosing the first printable character.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably that's simplest, yes. Let's document that we call rand(), and the caller can seed beforehand.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in cf8fc65.

Instead of taking a seed argument, just treat `int randomized`
like a flag. (I'd use `bool`, but the header consistently uses
`int` for boolean arguments.)
@@ -1047,7 +1047,7 @@ main(int argc, char *argv[])
}

if (generate_bounds > 0) {
if (!fsm_generate_matches(fsm, generate_bounds, fsm_generate_cb_printf_escaped, &opt)) {
if (!fsm_generate_matches(fsm, generate_bounds, 0, fsm_generate_cb_printf_escaped, &opt)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would make sense to expose this to the cli of course (and also for fsm(1)), but that can wait

@katef katef merged commit c375229 into main Oct 12, 2024
346 checks passed
@katef katef deleted the sv/randomized-generation-and-misc-fixes branch October 12, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants