On Sat, Jul 11, 2020 at 3:17 AM Lukas Bulwahn wrote: > > >> For Issue 5: Can you provide me (and the CC: the list) the list of > >> false positives (the commit hashes) you found for issue 5 on > >> EXPORT_SYMBOL? > > > > > > Here are the commit hashes for which the warning is issued: > > 54505a1e2083 > > 75d75b7a4d54 > > 8084c99b9af6 > > bfdaf029c9c9 > > dfd402a4c4ba > > > >> Can you also provide a short rationale/explanation for > >> each case that you considered a false positive? > > > > > > In each case the `EXPORT_SYMBOL()` is correctly written and the > variable/function to be exported > > is also inside the parentheses, still, we get the warning. Please let me > know if I am wrong here. > > I checked those warnings. Some of the patches are good cases to check if > we can improve the heuristics on EXPORT_SYMBOL(). > > E.g., commit bfdaf029c9c9 ("ia64: turn csum_partial_copy_from_user() into > csum_and_copy_from_user()") looks sound to me, and checkpatch.pl should > not really warn about that. > > Mrinal, maybe you can find out why checkpatch.pl believes that this case > is wrong and needs to be warned about? > Sir, Commit `54505a1e2083`, `8084c99b9af6` and `bfdaf029c9c9` have style issues. The documentation says to use `EXPORT_SYMBOL()` just after the closing bracket `}` of the function, it is exporting. In these commits `EXPORT SYMBOL()` is either defined at a point later or has been defined after leaving a blank line which is totally unneeded. However, commits `dfd402a4c4ba` and `75d75b7a4d54` look fine to me and have one thing in common, the `EXPORT_SYMBOL()` in these is used inside of a macro. checkpatch shouldn't warn about these. I have noticed checkpatch denoting wrong line numbers for certain files for several commits. For example, for the above commit `bfdaf029c9c9`, the following warning is issued: WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable #74: FILE: arch/ia64/lib/csum_partial_copy.c:122: +EXPORT_SYMBOL(csum_and_copy_from_user); but the file arch/ia64/lib/csum_partial_copy.c is only 113 lines long. Please let me know if you can reproduce this. Waiting for your insights. Thank you. > > > Lukas >