We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4841

Summary: Misplaced parenthesis in WIN_WindowProc
Product: SDL Reporter: Mathieu Eyraud <meyraud705>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: sylvain.becker
Version: 2.0.10   
Hardware: All   
OS: Windows (All)   
Attachments: Fix misplaced parathesis
clang-tidy-config.sh
modify_makefile.sh
improve tidy chack

Description Mathieu Eyraud 2019-10-21 11:49:03 UTC
Created attachment 3993 [details]
Fix misplaced parathesis

SDL_memcmp does not compare anything because of a misplaced parenthesis in https://hg.libsdl.org/SDL/file/74ac66d0e8ca/src/video/windows/SDL_windowsevents.c#l510 on line 510.
Comment 1 Sylvain 2019-10-23 07:01:41 UTC
Thanks good catch ! just wondering how you find this one ?
Added in https://hg.libsdl.org/SDL/rev/9d6107cbcd66

Btw this was introduced in:
https://hg.libsdl.org/SDL/rev/de4288fa5b0b
( WIN_WindowProc(), case WM_ACTIVATE comparing ClipCursor )
Comment 2 Mathieu Eyraud 2019-10-23 09:59:59 UTC
This is a warning in clang: -Wmemsize-comparison (enabled by default).

But because SDL use SDL_memcmp, compiling with clang does not trigger the warning.

However running clang-tidy triggers the warning because SDL defines SDL_memcmp to memcmp when __clang_analyzer__ is defined: https://hg.libsdl.org/SDL/file/6f0ec1079286/include/SDL_stdinc.h#l571 .
Comment 3 Sylvain 2019-10-26 13:06:24 UTC
Ok thanks for the info! 
I think I used clang-tidy some time ago.. I just re-tried, but it also gives me lots of code writing tips/warnings.

If you have a command line/script with all the interesting warning please copy paste it:)
Maybe this could be added to the Buildbots (https://buildbot.libsdl.org/waterfall) so that it's triggered on occasions.
Comment 4 Mathieu Eyraud 2019-10-28 12:41:59 UTC
Created attachment 4022 [details]
clang-tidy-config.sh
Comment 5 Mathieu Eyraud 2019-10-28 12:43:30 UTC
Created attachment 4023 [details]
modify_makefile.sh
Comment 6 Mathieu Eyraud 2019-10-28 12:46:17 UTC
Created attachment 4024 [details]
improve tidy chack

I attached the script I use to configure clang tidy (clang-tidy-config.sh).
It includes instructions to recreate it (for when the next version of clang, that will add new check, is out).
You can add '-' in front of a check to disable it. Then you run the script and place its output file in the root directory of the project. As long as the config file name is '.clang-tidy', it will be used automatically when you run clang-tidy.

To run clang-tidy on SDL:
mkdir ./build-clang-tidy
cd ./build-clang-tidy
./configure CC="clang-9" CFLAGS="-Wall -Wextra -Wno-sign-compare -Wno-unused-parameter -Wformat=2 "

Then I modify the Makefile with 'sed' (modify_makefile.sh):
 - set 'CC' value to 'clang-tidy'
 - modify 'all' dependencies to skip rules that create so/dll files.
 - modify object rules for clang-tidy syntax and adding 'touch $@' so that next time you run clang-tidy only modified files will be checked.

make -j4 1>./tidy.log

I also attached a patch to improve clang-tidy result:
 - defining some SDL function to the libc one, like what is done in SDL_stdinc.h
 - let clang-tidy know that SDL_SetError always return -1.