Comments (15)
Thanks for your report and the heads-up to use a static analyzer like clang-tidy (edited, was: -format). As for your reports:
- https://github.com/fltk/fltk/blob/master/src/drivers/X11/Fl_X11_Screen_Driver.cxx#L971 : I think this is one for our specialist @ManoloFLTK.
- https://github.com/fltk/fltk/blob/master/src/drivers/X11/Fl_X11_Window_Driver.cxx#L237 : This is not a fault. X, Y, W, and H are reference arguments that return values. Yes, we could initialize them with 0 to prevent warnings. I'll take a look...
- https://github.com/fltk/fltk/blob/master/src/drivers/X11/Fl_X11_Screen_Driver.cxx#L1241 : yes, you are right, it's not a bug. I think the following change would make it clear:
p = fgets(line, sizeof(line), in);
if (p && strstr(line, "<monitors version=\"2\">")) {
i.e. add p &&
in the condition. I'll take this one as well.
PS: I checked the entire source with clang-tidy (default, no special setup), and I found some issues. It doesn't look too bad but there are some noteworthy warnings I'll check and take care of. Thanks again.
from fltk.
Could be nice to run a static analyzer after each submission / pr request to master ...
from fltk.
Part 3 is fixed in commit 2e6a4eb.
Part 2 needs a little more work since there are many similar places in the code.
from fltk.
Regarding part 1:
The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage. https://github.com/fltk/fltk/blob/master/src/drivers/X11/Fl_X11_Screen_Driver.cxx#L971
I can see and fix the issue, but unfortunately my version of clang-tidy (6.0) doesn't issue this warning. I'm using:
clang-tidy $1 -- -I$SRCDIR -DFL_LIBRARY `$BLDDIR/fltk-config --use-images --use-gl --cxxflags`
where $1 is the filename, $SRCDIR is the FLTK source directory, and $BLDDIR is the FLTK build directory which may be different if using CMake. I'm using the default config.
Pavel, can you please elaborate on how you are calling clang-tidy?
from fltk.
@Albrecht-S I'm not sure about the flags as I use Qt Creator to develop. It has predefined sets of flags, so I used one of these...
from fltk.
@shlyakpavel Thanks for your reply anyway. Do you know your clang and clang-tidy versions?
from fltk.
./clang --version
clang version 7.0.0 (git://code.qt.io/clang/clang.git 1817513d4f3a2e4e26be124dbe395340f798fd51) (git://code.qt.io/clang/llvm.git 287d3ae13f372cfa7f30801199d02a99b60d563b)
Target: x86_64-unknown-linux-gnu
Thread model: posix
clang-tidy mask was -*,clang-analyzer-*
from fltk.
However, I personally prefer to use PVS studio instead of clang-tidy. According to my experience it always finds much more relevant problems than clang-based tools do. They are commercial project but one can ask 'em for a trial license :)
from fltk.
Apparently PVS Studio has a command line as well, allowing for automation (and avoiding GUI IDEs):
https://www.viva64.com/en/m/0035/
from fltk.
Regarding part 1:
The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage. https://github.com/fltk/fltk/blob/master/src/drivers/X11/Fl_X11_Screen_Driver.cxx#L971
That is May 2002 code by M.R.Sweet which decodes the output of the XGetImage
Xlib function into an RGB byte array. I have no knowledge of the algorithm which seems
somewhat complex. The statements containing index_mask at lines #971 and #974
apparently should just be removed because the value of that variable isn't used anywhere
in the program path that goes through these lines. Albrecht: is that your finding too?
from fltk.
No, I don't think so. I'd rather believe that the correct fix would be to add an initialization in the loop statement at line no. 961 like this one:
for (x = image->width, line_ptr = line, index_mask = 192, index_shift = 6;
This seems to be what is used inside the loop. But I'd need to check this - it's just my impression from looking at the code...
@ManoloFLTK Why did you close the issue? By accident? Sorry for mentioning you explicitly and for assigning the issue to you. I'll look into it and see if the above fix is correct. Feel free to unassign yourself.
from fltk.
@ManoloFLTK Sorry again, your proposal is probably correct since the index_mask value is never used.
Still looking into it...
from fltk.
@ManoloFLTK Why did you close the issue? By accident?
That must have been an accident, while discovering github.
from fltk.
OK, part 1 is now also fixed in commit 59cd39a.
I removed the questionable variable in this case statement since it is not used as pointed out by @ManoloFLTK. Thanks.
from fltk.
Regarding part 2 (uninitialized ints): I opened a new issue (#6) with a more general description of the issue.
I'm closing this one now. Thanks for all contributions.
from fltk.
Related Issues (20)
- CMake's USE_SYSTEM libs does not work on Windows' MSVC HOT 4
- Fl_Terminal control character handling HOT 9
- Fl_Gl_Window glViewport() clips drawing on Windows/Mingw64 HOT 6
- Build using a GLU from a non-standard location HOT 5
- Naseem HOT 2
- FLUID: compilation warnings HOT 1
- Wayland use of alpha in glClearColor/glClear() -- bug or misfeature? HOT 13
- Building OpenGL Examples on Windows with MSVC does not work HOT 5
- Consider adding compile-definition "FL_DLL" to MSVC 2022 builds with shared libraries HOT 6
- macOS: Generating fltk.framework HOT 13
- Avoiding wrong includes (only include from FLTK's repository not from CMAKE_PREFIX_PATH) HOT 5
- FLTK 1.4: X11 Clipping bugs HOT 8
- Fluid/MSVC compilation warnings HOT 1
- FLTK hangs when a GL visible subwindow goes partially below the screen on Wayland-Gnome HOT 14
- macOS regression with glwindow, glsubwindow and tile. HOT 14
- Selected Fl_Menu_Item's are not drawn with selection color background under Windows HOT 8
- Cannot build project with CMake (add_library cannot create ALIAS target "fltk" because target "fltk::fltk" is imported but not globally visible) HOT 7
- Fl_Tree handler might refer to a null root causing a crash HOT 4
- Fl_Tree doesn't use system colors HOT 6
- Wayland: subwindow redraw issues from timeouts with NVidia driver HOT 26
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fltk.