-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
libevent: Fuzzing Coverage Expansion #2 #11377
Conversation
Performing changes according to libevent maintainer's comments
viktoriia-lsg has previously contributed to projects/libevent. The previous PR was #11257 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit, could we leave the printf
calls out?
@DavidKorczynski sorry for late response. @azat asked to add these checks, so |
@@ -56,30 +59,48 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { | |||
} | |||
bev1 = pair[0]; | |||
bev2 = pair[1]; | |||
bufferevent_pair_get_partner(bev1); | |||
if (!bufferevent_pair_get_partner(bev1)) { | |||
printf("Bufferevent partner is not found\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will Ci fails if this printf will be triggered? I think assert is OK, I saw discussion in this PR about it may introduce lots of crashes, but this should not fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@azat No. Crashes won't be generated for printf, but for assert will. Do you want to change it only for the specified test case or for other cases as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use assert everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for confirmation, I will prepare another pull request soon.
Hi! This pull request makes some changes to the previous pull request #11377. In detail, all **printf** statements were replaced with assertions. @DavidKorczynski please check when you have a time. Fyi @azat.
Hi! This pull request extends the previous pull request #11257 and brings some improvements.