This is the result of running cppcheck 1.35 with the --all parm against the august 27th Git tree:
[../wine-git/dlls/dbghelp/msc.c:88]: (possible error) Array index out of bounds [../wine-git/dlls/dbghelp/msc.c:89]: (possible error) Array index out of bounds [../wine-git/dlls/msvcrt/tests/file.c:982]: (error) Deallocating a deallocated pointer: stream2 [../wine-git/dlls/msvcrt/tests/file.c:966]: (error) Resource leak: stream3 [../wine-git/dlls/msvcrt/tests/file.c:973]: (error) Resource leak: stream4 [../wine-git/dlls/msvcrt/tests/heap.c:433]: (possible error) Memory leak: mem [../wine-git/dlls/ntdll/server.c:802]: (error) Resource leak: fd [../wine-git/dlls/rpcrt4/tests/server.c:1189]: (possible error) Array index out of bounds [../wine-git/dlls/shell32/cpanelfolder.c:562]: (error) Possible null pointer dereference: rgfInOut [../wine-git/dlls/shell32/shfldr_desktop.c:437]: (error) Possible null pointer dereference: rgfInOut [../wine-git/dlls/shell32/shfldr_fs.c:577]: (error) Possible null pointer dereference: rgfInOut [../wine-git/dlls/shell32/shfldr_mycomp.c:474]: (error) Possible null pointer dereference: rgfInOut [../wine-git/dlls/shell32/shfldr_netplaces.c:352]: (error) Possible null pointer dereference: rgfInOut [../wine-git/dlls/user32/tests/msg.c:63]: (error) Invalid number of character ({). Can't process file. [../wine-git/dlls/winealsa.drv/waveinit.c:745]: (possible error) Buffer overrun [../wine-git/dlls/wined3d/arb_program_shader.c:907]: (possible error) Buffer overrun [../wine-git/dlls/wined3d/arb_program_shader.c:915]: (possible error) Buffer overrun [../wine-git/dlls/wined3d/glsl_shader.c:3416]: (possible error) Buffer overrun [../wine-git/dlls/wined3d/glsl_shader.c:3418]: (possible error) Buffer overrun [../wine-git/dlls/wined3d/glsl_shader.c:3519]: (possible error) Buffer overrun [../wine-git/dlls/wined3d/glsl_shader.c:3521]: (possible error) Buffer overrun [../wine-git/dlls/wineoss.drv/mixer.c:1458]: (possible error) Buffer overrun [../wine-git/dlls/wineps.drv/init.c:270]: (error) Possible null pointer dereference: dmW [../wine-git/programs/oleview/pane.c:152]: (error) Possible null pointer dereference: hWndCreated [../wine-git/programs/winetest/main.c:114]: (possible error) Buffer overrun [../wine-git/programs/winetest/main.c:116]: (possible error) Buffer overrun [../wine-git/programs/winetest/main.c:119]: (possible error) Buffer overrun [../wine-git/programs/winetest/main.c:121]: (possible error) Buffer overrun [../wine-git/server/file.c:235]: (possible error) Buffer overrun [../wine-git/tools/fnt2bdf.c:656]: (error) Resource leak: fd [../wine-git/tools/fnt2fon.c:303]: (error) Memory leak: file_lens [../wine-git/tools/makedep.c:337]: (error) Resource leak: file [../wine-git/tools/widl/write_msft.c:2540]: (error) Deallocating a deallocated pointer: fd [../wine-git/tools/winedump/pe.c:1549]: (possible error) Memory leak: map
Chris
On Thu, Aug 27, 2009 at 3:52 PM, chris ahrendtcelticht32@yahoo.com wrote:
False positive, apparently the numbers are hardcoded as: 72 char msg[128]; 88 msg[10 + 3 * 16] = ' '; // = 58<127 89 msg[10 + 3 * 16 + 1 + 16] = '\0'; // = 75<127
It doesn't like the ternary operator? These lines are TRACE lines with one of the args being ``rgfInOut ? *rgfInOut : 0''. False positive.
739 char defaultpcmname[256]; 745 sprintf(defaultpcmname, "default");
Something is wrong with cppcheck... False positive.
Not checking those - i don't even pretend to understand how modern graphics works.
[../wine-git/dlls/wineoss.drv/mixer.c:1458]: (possible error) Buffer overrun
So...it picks 1455 char name[32]; 1458 sprintf(name, "/dev/mixer");
as an error, but not:
1460 sprintf(name, "/dev/mixer%d", i);
False positive.
[../wine-git/dlls/wineps.drv/init.c:270]: (error) Possible null pointer dereference: dmW
This one is actually a bug, the null check is below this line. All the callers check for nulls, though.
[../wine-git/programs/oleview/pane.c:152]: (error) Possible null pointer dereference: hWndCreated
Also a bug, and a very real one. Coincidentally, the null check on the next line is also wrong (should be if (!*hWndCreated) )
More of sprintf with just a string nonsense. False positive.
[../wine-git/server/file.c:235]: (possible error) Buffer overrun
Also sprintf nonsense, but slightly more dangerous. The buffer is declared with [16] and the string is of length 14+1, so a few more bytes wouldn't hurt. :)
Chris
If someone could send patches for the few bugs that would be nice.
Chris - cppcheck is clearly crazy about sprintf's and ternary operators. You might want to report that.
Mike.
Mike Kaplinskiy wrote:
Why not just say 58 instead of the complex math?
89 msg[10 + 3 * 16 + 1 + 16] = '\0'; // = 75<127
Same here. Why not 75 instead of the math formula?
This is relying on a certain path for math which may or may not be true (RPN anyone?) It was best practice to use constants vice complex math to determine array locations or to set and test a variable before the call.
I could see this if a variable was used in the math formula that was checked for out of bounds, but this is not the case.
James McKenzie
"James McKenzie" jjmckenzie51@earthlink.net wrote:
Because that's done that way to make it easier to understand the logic for the programmer. If a tool can't cope with it - that tool is broken.
2009/8/28 Dmitry Timoshkov dmitry@codeweavers.com:
I just took a look at the code, and I agree with Dmitry. In context, these lines appear immediately after a loop structure that is populating the array. The "10 + 3 * 16" calculated constants make perfect sense in the context of the loop (note that there is also a "+ 0" on line 82). Changing these to single constant values would make it much harder for the casual observer to read.
Note that the compiler will optimise this to a single constant anyway.
Either way, it's not an actual *bug* unless the values are wrong.
On Fri, Aug 28, 2009 at 4:06 AM, Ben Kleinshacklein@gmail.com wrote:
Changing these to single constant values would make it much harder for the casual observer to read.
That's what comments are for.
Actually, comments are for code that's not understandable without them. The code is more readable as it is, IMHO, than it would be by replacing the expression--perfectly natural in context--with its evaluation, or with a symbolic constant. --Juan
Juan Lang wrote:
Actually, I'm with Austin on this one. A comment on the line that the code was in would be most helpful and would explain the use of the complex math formulae rather than having to guess at what is going on. That is why comments exist. To prevent guessing why certain code is why is the way it was written and for others (remember you can get run over by a bus, get hit by a car, get Swine Flu and be out for weeks) to not have to guess. I believe in overcommenting rather than under. BTW, that is what I was taught and is ITIL BBP.
James McKenzie
-
Austin English -
Ben Klein -
chris ahrendt -
Dmitry Timoshkov -
James McKenzie -
Juan Lang -
Mike Kaplinskiy