search

dbus-fuzzer / dfuzzer

D-Bus fuzzer
GNU General Public License v3.0
37 stars 10 forks source link

Extract all public stuff from `dfuzzer.h` and drop the file completely #117

Closed mrc0mmand closed 2 years ago

mrc0mmand commented 2 years ago

This way we should be able to link against all fuzz/rand related dfuzzer stuff in future unit tests.

Note: this is currently based on top of #115, since I want to see how CI is going to like it.

mrc0mmand commented 2 years ago

Oh well, I guess no CI for me https://www.githubstatus.com/incidents/qzmhbwp7f6nn :-)

evverx commented 2 years ago

The first two commits look good to me (though I'm not sure df_fuzz_set_buffer_length should be the only place where fuzz_buffer_length is checked). I think in addition to the assertion it should be verified somewhere along with buf_length < MIN_BUFFER_LENGTH with a clear warning/error.

The last commit is kind of controversial. I think ideally the suppression part should go to a separate file to make it easier to test it with unit tests of some kind.

mrc0mmand commented 2 years ago

The first two commits look good to me (though I'm not sure df_fuzz_set_buffer_length should be the only place where fuzz_buffer_length is checked). I think in addition to the assertion it should be verified somewhere along with buf_length < MIN_BUFFER_LENGTH with a clear warning/error.

Whoops, I thought we already check both MIN a MAX_BUFFER_LENGTH, will fix.

The last commit is kind of controversial. I think ideally the suppression part should go to a separate file to make it easier to test it with unit tests of some kind.

That's a good point, let me fix that.

mrc0mmand commented 2 years ago

Note for future RFE: we should probably check the CI logs somehow for sanitizer errors (or change the exit code & check for it), since I almost missed https://github.com/dbus-fuzzer/dfuzzer/runs/7171253508?check_suite_focus=true#step:5:1001

mrc0mmand commented 2 years ago

This should be, hopefully, ready. The last commits are in a "wrong logical order", since the genius me did all the work on the latest commit and now switching them around introduces quite a lot of conflicts - hopefully it's not a big issue.

evverx commented 2 years ago

we should probably check the CI logs somehow for sanitizer errors

If I understand correctly I think set -o pipefail could have helped to catch that.