Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2012443002:
[libfuzzer] Add fuzzer for third_party/hunspell with dictionary. (Closed)
|
Created:
4 years, 7 months ago by mmoroz Modified:
4 years, 6 months ago Reviewers:
Oliver Chang, groby-ooo-7-16, please use gerrit instead, aizatsky, kcc2, Paweł Hajdan Jr. CC:
chromium-reviews, groby+spellwatch_chromium.org, rlp+watch_chromium.org, rouslan+spell_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[libfuzzer] Add fuzzer for third_party/hunspell with dictionary.
Ported from google3 with minor changes.
R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org, phajdan.jr@chromium.org
BUG=539572
Committed: https://crrev.com/22d7c9d03b17a725a5abd968af8d64311e89b0b8
Cr-Commit-Position: refs/heads/master@{#398274}
Patch Set 1 #Patch Set 2 : Forgot to add line endings into hunspell dictionary. #
Total comments: 3
Patch Set 3 : Add #include guard and license block to hunspell_fuzzer_dictionary.h. #
Total comments: 22
Patch Set 4 : Rouslan's comments addressed. #
Total comments: 8
Patch Set 5 : Addressing more comments. #
Total comments: 4
Patch Set 6 : Remove redundant newlines. #Patch Set 7 : Remove dictionary entries with 'wstring' substring. #Patch Set 8 : Add licence to checklicenses.py. #
Created: 4 years, 6 months ago
(Patch set is too large to download)
Messages
Total messages: 38 (15 generated)
Description was changed from ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org BUG=539572 ========== to ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org BUG=539572 ==========
rouslan@chromium.org changed reviewers: + groby@chromium.org - rouslan@chromium.org
lgtm
https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:5: // This is "third_party/hunspell_dictionaries/en_US.dic". Does the license of this file agree with en_US.dic? Should you keep the original license if you simply "repackage" the file?
Thanks Mike for catching this. https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:5: // This is "third_party/hunspell_dictionaries/en_US.dic". On 2016/05/31 18:49:05, aizatsky wrote: > Does the license of this file agree with en_US.dic? > > Should you keep the original license if you simply "repackage" the file? Good question! I've added a GPL license block from third_party/hunspell_dictionaries.
rouslan@chromium.org changed reviewers: + rouslan@chromium.org
https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:5: // This is "third_party/hunspell_dictionaries/en_US.dic". On 2016/06/01 09:38:27, mmoroz wrote: > On 2016/05/31 18:49:05, aizatsky wrote: > > Does the license of this file agree with en_US.dic? > > > > Should you keep the original license if you simply "repackage" the file? > > Good question! I've added a GPL license block from > third_party/hunspell_dictionaries. Keep in mind that the block you've added is not GPL, which cannot be used in Chrome. You've added GPL/LGPL/MPL tri-license, which is OK to use.
On 2016/06/01 16:35:21, Rouslan (ツ) wrote: > https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... > File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): > > https://codereview.chromium.org/2012443002/diff/20001/third_party/hunspell/fu... > third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:5: // This is > "third_party/hunspell_dictionaries/en_US.dic". > On 2016/06/01 09:38:27, mmoroz wrote: > > On 2016/05/31 18:49:05, aizatsky wrote: > > > Does the license of this file agree with en_US.dic? > > > > > > Should you keep the original license if you simply "repackage" the file? > > > > Good question! I've added a GPL license block from > > third_party/hunspell_dictionaries. > > Keep in mind that the block you've added is not GPL, which cannot be used in > Chrome. You've added GPL/LGPL/MPL tri-license, which is OK to use. Thanks for letting me know. I thought that 'GPL/LGPL/MPL tri-license' implies any of these licenses, but it's definitely incorrect to call it GPL. Could you please review the CL, since you are one of the OWNERS for hunspell?
https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/BU... File third_party/hunspell/BUILD.gn (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/BU... third_party/hunspell/BUILD.gn:117: "fuzz/hunspell_fuzzer.cc", Sources should include the header files, too. You would want changes in your headers to cause a rebuild as well. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/BU... third_party/hunspell/BUILD.gn:122: dict = "fuzz/hunspell.dict" Either here or inside of hunspell.dict, explain that this "dict" is not a spellcheck dictionary. It's confusing, because you're fuzzing a spellcheck library, which has its own definition of what a dictionary is. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell.dict (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:1: # Has been generated from third_party/hunspell/tests/suggestiontest \ Chrome license header + license of List_of_common_misspellings.txt ? https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:10: #include <string> This include should be next to <stdint.h> https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:12: #include "hunspell_fuzzer_dictionary.h" All includes should have full path. #include "third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h" https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:14: static Hunspell *CreateHunspell() { This function is used only once. You should inline it. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:5: #ifndef HUNSPELL_FUZZ_HUNSPELL_FUZZER_DICTIONARY_H_ Header guard should include the full path name. #ifndef THIRD_PARTY_HUNSPELL_FUZZ_HUNSPELL_FUZZER_DICTIONARY_H_ https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:29: // TODO: Add other dictionaries if it affects coverage while fuzzing. All TODOs should have a username and a bug #. // TODO(username): Task description here. http://crbug.com/12345. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:30: static const char kHunspell_Dictionary[] = 1) Static constants should be camel cased without underscores. 2) It's best to make this an extern with the actual string in hunspell_fuzzer_dictionary.cc, a la https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil... extern const char kHunspellDictionary[];
Thanks Rouslan! Please take a look :) https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/BU... File third_party/hunspell/BUILD.gn (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/BU... third_party/hunspell/BUILD.gn:117: "fuzz/hunspell_fuzzer.cc", On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > Sources should include the header files, too. You would want changes in your > headers to cause a rebuild as well. Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/BU... third_party/hunspell/BUILD.gn:122: dict = "fuzz/hunspell.dict" On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > Either here or inside of hunspell.dict, explain that this "dict" is not a > spellcheck dictionary. It's confusing, because you're fuzzing a spellcheck > library, which has its own definition of what a dictionary is. Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell.dict (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:1: # Has been generated from third_party/hunspell/tests/suggestiontest \ On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > Chrome license header + license of List_of_common_misspellings.txt ? Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:10: #include <string> On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > This include should be next to <stdint.h> Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:12: #include "hunspell_fuzzer_dictionary.h" On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > All includes should have full path. > > #include "third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h" Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:14: static Hunspell *CreateHunspell() { On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > This function is used only once. You should inline it. We disable inlining for fuzzers, so I guess it won't make sense. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:5: #ifndef HUNSPELL_FUZZ_HUNSPELL_FUZZER_DICTIONARY_H_ On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > Header guard should include the full path name. > > #ifndef THIRD_PARTY_HUNSPELL_FUZZ_HUNSPELL_FUZZER_DICTIONARY_H_ Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:29: // TODO: Add other dictionaries if it affects coverage while fuzzing. On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > All TODOs should have a username and a bug #. > > // TODO(username): Task description here. http://crbug.com/12345. Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:30: static const char kHunspell_Dictionary[] = On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > 1) Static constants should be camel cased without underscores. > 2) It's best to make this an extern with the actual string in > hunspell_fuzzer_dictionary.cc, a la > > https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil... > > extern const char kHunspellDictionary[]; Thanks for the suggestion, but I think it won't be comfortable to have a 50K-lines constant in the same source file with the target function. This is why this auxiliary header has been created actually. Name fixed.
https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:14: static Hunspell *CreateHunspell() { On 2016/06/02 10:54:36, mmoroz wrote: > On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > > This function is used only once. You should inline it. > > We disable inlining for fuzzers, so I guess it won't make sense. Sorry, when I said "inline", I meant this: static Hunspell* hunspell = new Hunspell( reinterpret_cast<const unsigned char*>(kHunspellDictionary), sizeof kHunspellDictionary); https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:30: static const char kHunspell_Dictionary[] = On 2016/06/02 10:54:36, mmoroz wrote: > On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > > 1) Static constants should be camel cased without underscores. > > 2) It's best to make this an extern with the actual string in > > hunspell_fuzzer_dictionary.cc, a la > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil... > > > > extern const char kHunspellDictionary[]; > > Thanks for the suggestion, but I think it won't be comfortable to have a > 50K-lines constant in the same source file with the target function. This is why > this auxiliary header has been created actually. I agree that kHunspellDicitonary should not be in hunspell_fuzzer.cc. I am suggesting that the "extern" be in hunspell_fuzzer_dicitonary.h and the actual 50k-lines constant should be in hunspell_fuzzer_dictionary.cc. > Name fixed. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell.dict (right): https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:6: # It has been generated from third_party/hunspell/tests/suggestiontest \ Can you specify that command that you used for generation? That way updates to hunspell library will be easier for maintainers. If you did it by hand, you can say "generated manually". https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:6: # It has been generated from third_party/hunspell/tests/suggestiontest \ I would prefer you to not break paths and URLs for 80 character rule. That makes copy-pasting the URLs easier. For example: It has been generated from: third_party/hunspell/tests/suggestiontest/List_of_common_misspellings.txt. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:9: # source: http://en.wikipedia.org/wiki/Wikipedia:Lists_of_common_misspellings \ This "source" line is confusing. It comes verbatim from List_of_common_misspellings.txt in hunspell, but that's not clear. If someone did not see the "generated from" line, they might think you're including code from Wikipedia yourself here. You are, instead, including code from hunspell. Remove this line. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:16: sizeof(kHunspellDictionary)); When used with expressions (not types), the parens are not necessary: sizeof kHunspellDicitonary See http://en.cppreference.com/w/cpp/language/sizeof
Thanks again! https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:14: static Hunspell *CreateHunspell() { On 2016/06/02 16:03:50, Rouslan (ツ) wrote: > On 2016/06/02 10:54:36, mmoroz wrote: > > On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > > > This function is used only once. You should inline it. > > > > We disable inlining for fuzzers, so I guess it won't make sense. > > Sorry, when I said "inline", I meant this: > > static Hunspell* hunspell = new Hunspell( > reinterpret_cast<const unsigned char*>(kHunspellDictionary), > sizeof kHunspellDictionary); Done. https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h (right): https://codereview.chromium.org/2012443002/diff/30001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:30: static const char kHunspell_Dictionary[] = On 2016/06/02 16:03:50, Rouslan (ツ) wrote: > On 2016/06/02 10:54:36, mmoroz wrote: > > On 2016/06/01 17:09:17, Rouslan (ツ) wrote: > > > 1) Static constants should be camel cased without underscores. > > > 2) It's best to make this an extern with the actual string in > > > hunspell_fuzzer_dictionary.cc, a la > > > > > > > > > https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil... > > > > > > extern const char kHunspellDictionary[]; > > > > Thanks for the suggestion, but I think it won't be comfortable to have a > > 50K-lines constant in the same source file with the target function. This is > why > > this auxiliary header has been created actually. > > I agree that kHunspellDicitonary should not be in hunspell_fuzzer.cc. > > I am suggesting that the "extern" be in hunspell_fuzzer_dicitonary.h and the > actual 50k-lines constant should be in hunspell_fuzzer_dictionary.cc. > > > Name fixed. > According to the guidelines we should keep headers self-contained (https://google.github.io/styleguide/cppguide.html#Self_contained_Headers). Since we have only a constant here, I think it would be better to leave it here. For example, if we add another fuzzer, it will be enough to include the header only, which seems simpler than taking care of .h + .cc. May be I understand meaning of "self-contained" incorrectly. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell.dict (right): https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:6: # It has been generated from third_party/hunspell/tests/suggestiontest \ On 2016/06/02 16:03:50, Rouslan (ツ) wrote: > Can you specify that command that you used for generation? That way updates to > hunspell library will be easier for maintainers. If you did it by hand, you can > say "generated manually". Done. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:6: # It has been generated from third_party/hunspell/tests/suggestiontest \ On 2016/06/02 16:03:50, Rouslan (ツ) wrote: > I would prefer you to not break paths and URLs for 80 character rule. That makes > copy-pasting the URLs easier. For example: > > It has been generated from: > > third_party/hunspell/tests/suggestiontest/List_of_common_misspellings.txt. Done. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell.dict:9: # source: http://en.wikipedia.org/wiki/Wikipedia:Lists_of_common_misspellings \ On 2016/06/02 16:03:50, Rouslan (ツ) wrote: > This "source" line is confusing. It comes verbatim from > List_of_common_misspellings.txt in hunspell, but that's not clear. If someone > did not see the "generated from" line, they might think you're including code > from Wikipedia yourself here. You are, instead, including code from hunspell. > Remove this line. Done. https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/40001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:16: sizeof(kHunspellDictionary)); On 2016/06/02 16:03:50, Rouslan (ツ) wrote: > When used with expressions (not types), the parens are not necessary: > > sizeof kHunspellDicitonary > > See http://en.cppreference.com/w/cpp/language/sizeof Yeah, but it is absolutely legit since "expression" ~~ "(expression)", § 5.1.1.6 at ISO/IEC 14882:2011. There is a plenty of ways to make code less readable, good thing that usually we are not required to follow them :)
lgtm % comments https://codereview.chromium.org/2012443002/diff/50001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/50001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:10: No newline here. https://codereview.chromium.org/2012443002/diff/50001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:12: Only 1 newline here.
Thanks! https://codereview.chromium.org/2012443002/diff/50001/third_party/hunspell/fu... File third_party/hunspell/fuzz/hunspell_fuzzer.cc (right): https://codereview.chromium.org/2012443002/diff/50001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:10: On 2016/06/03 17:52:45, Rouslan (ツ) wrote: > No newline here. Done. https://codereview.chromium.org/2012443002/diff/50001/third_party/hunspell/fu... third_party/hunspell/fuzz/hunspell_fuzzer.cc:12: On 2016/06/03 17:52:45, Rouslan (ツ) wrote: > Only 1 newline here. Done.
The CQ bit was checked by mmoroz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ochang@chromium.org, rouslan@chromium.org Link to the patchset: https://codereview.chromium.org/2012443002/#ps50002 (title: "Remove redundant newlines.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2012443002/50002
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Failure message does not make much sense, because you're not using wstrings.
** Presubmit Warnings **
New code should not use wstrings. If you are calling a cross-platform API that
accepts a wstring, fix the API.
third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:14487
third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:20644
Perhaps you can truncate third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h
to ~1k words? That would make managing your word list easier and should not
affect the fuzzer, afaik.
The CQ bit was checked by mmoroz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org, ochang@chromium.org Link to the patchset: https://codereview.chromium.org/2012443002/#ps60001 (title: "Remove dictionary entries with 'wstring' substring.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2012443002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/06/03 18:29:28, Rouslan (ツ) wrote: > Failure message does not make much sense, because you're not using wstrings. > > ** Presubmit Warnings ** > New code should not use wstrings. If you are calling a cross-platform API that > accepts a wstring, fix the API. > third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:14487 > third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h:20644 > > Perhaps you can truncate third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h > to ~1k words? That would make managing your word list easier and should not > affect the fuzzer, afaik. Good point, but I guess that it would be better to use some real dictionary. I've removed two strings with "wstring" occurrences. Looks like it helped, now there is a license checking error: checklicenses (with patch) failures: third_party/hunspell/fuzz/hunspell_fuzzer_dictionary.h: MPL (v1.1) BSD-like LGPL (v2.1 or later)
Description was changed from ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org BUG=539572 ========== to ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org, phajdan.jr@chromium.org BUG=539572 ==========
mmoroz@chromium.org changed reviewers: + phajdan.jr@chromium.org
+Pawel for 'tools/checklicenses/checklicenses.py' update. Not sure if it is the best solution though.
LGTM
The CQ bit was checked by mmoroz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org, ochang@chromium.org Link to the patchset: https://codereview.chromium.org/2012443002/#ps70001 (title: "Add licence to checklicenses.py.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2012443002/70001
Message was sent while issue was closed.
Description was changed from ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org, phajdan.jr@chromium.org BUG=539572 ========== to ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org, phajdan.jr@chromium.org BUG=539572 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:70001)
Message was sent while issue was closed.
Description was changed from ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org, phajdan.jr@chromium.org BUG=539572 ========== to ========== [libfuzzer] Add fuzzer for third_party/hunspell with dictionary. Ported from google3 with minor changes. R=aizatsky@chromium.org, kcc@chromium.org, ochang@chromium.org, rouslan@chromium.org, phajdan.jr@chromium.org BUG=539572 Committed: https://crrev.com/22d7c9d03b17a725a5abd968af8d64311e89b0b8 Cr-Commit-Position: refs/heads/master@{#398274} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/22d7c9d03b17a725a5abd968af8d64311e89b0b8 Cr-Commit-Position: refs/heads/master@{#398274} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
