Add new project: kcodecs #2127
Add new project: kcodecs #2127
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
|
CLAs look good, thanks! |
|
Putting this in sheriff's queue. |
|
@tsdgeos, did you follow the New Project Guide, especially the Testing Locally section? |
|
|
||
| cd $SRC | ||
| cd kcodecs | ||
| cmake . -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=$SRC/qtbase |
There was a problem hiding this comment.
We usually run fuzzers in Release mode, because asserts are often very noisy. However, if asserts are not firing too often, it's nice to have them enabled. Might be worth running the fuzzer locally for several minutes / hours just to see if it's not triggering asserts too often.
There was a problem hiding this comment.
Done it's not triggering asserts.
projects/kcodecs/build.sh
Outdated
| make -j$(nproc) VERBOSE=1 | ||
|
|
||
|
|
||
| $CXX $CXXFLAGS -fPIC -std=c++11 $SRC/kcodecs_fuzzer.cc -o $OUT/kcodecs_fuzzer -I $SRC/qtbase/include/QtCore/ -I $SRC/qtbase/include/ -I $SRC/kcodecs/src -I $SRC/kcodecs/src/probers -L $SRC/qtbase/lib -L $SRC/kcodecs/lib -lQt5Core -lm -lqtpcre2 -ldl -lpthread -lFuzzingEngine -lKF5Codecs |
There was a problem hiding this comment.
nit: could you please format this into multi-line command for better readability?
There was a problem hiding this comment.
it seems like you need a nicer editor that helps with long lines readability, but i'll try to do it even if that means have to sit during a 20 minute build wasting my time to make sure i didn't break the build while making a line arguably nicer
| new kencodingprober::UnicodeGroupProber() | ||
| }; | ||
|
|
||
| for (kencodingprober::nsCharSetProber *p : probers) { |
There was a problem hiding this comment.
I'd strongly recommend splitting this into different fuzz targets. Should be easy to implement based on a macro which you can iterate over in the build script. For an example, see expat fuzz target and the build script. Splitting different data types would very likely give a better coverage, as every fuzz target will be focusing on a certain format only (we did some experiments).
There was a problem hiding this comment.
I would rather not do that.
|
|
||
| const QByteArray ba((const char *)data, size); | ||
| const QVector<const char*> codecs = { "base64", "quoted-printable", "b", "q", "x-kmime-rfc2231", "x-uuencode" }; | ||
| for (const char *codecName : codecs) { |
There was a problem hiding this comment.
This is fine to iterate through, I like it.
Yes, this is the 3rd project i submit/contribute to, i have tested them all. |
Fantastic! Just want to clarify that as quite often contributors don't really test stuff properly and things break later on :) |
|
Updated to remove whitespace, add header to build.sh and break long line |
|
Thanks, @tsdgeos! SInce it's not the first project you're adding, you might've already heard of Ideal Integration. An extra benefit of it is that maintainers store the sources / build rules in their own repo and don't need to comply with our code review requests :) That also allows the maintainers to apply for a greater integration reward and saves some time on our end. |
|
I'm not sure the test of developers would want to have the sources in the upstream repo, i'll have to talk with them. Thanks for mentioning it :) |
I'm part of KDE's security team
https://www.kde.org/info/security/