Hi all, Clang found a potential bug. In /src/extension/internal/wmf-inout.cpp, line 1408: int32_t width, height, colortype, numCt, invert; if( (iUsage != U_DIB_RGB_COLORS) || !(dibparams = wget_DIB_params( // this returns pointers and values, but allocates no memory dib, &px, &ct, &numCt, &width, &height, &colortype, &invert )) ){
Now if the first condition is true ("(iUsage != U_DIB_RGB_COLORS)") the second part will not be executed, and thus all those ref'd variables will go un-initialized. I'd appreciate it if someone with more knowledge about wmf-inout.cpp would have a look and fix the code (it's pretty ugly to put the initialization call in the if-condition...)
Thanks! Johan
On 05-Nov-2013 12:31, Johan Engelen wrote:
Hi all, Clang found a potential bug. In /src/extension/internal/wmf-inout.cpp, line 1408: int32_t width, height, colortype, numCt, invert; if( (iUsage != U_DIB_RGB_COLORS) || !(dibparams = wget_DIB_params( // this returns pointers and values, but allocates no memory dib, &px, &ct, &numCt, &width, &height, &colortype, &invert )) ){
Now if the first condition is true ("(iUsage != U_DIB_RGB_COLORS)") the second part will not be executed,
That is intentional. What is the full message? Does it go away if this change is made to line 1395?
int dibparams;
to
int dibparams = U_BI_RGB;
Regards,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
On 6-11-2013 1:27, mathog wrote:
On 05-Nov-2013 12:31, Johan Engelen wrote:
Hi all, Clang found a potential bug. In /src/extension/internal/wmf-inout.cpp, line 1408: int32_t width, height, colortype, numCt, invert; if( (iUsage != U_DIB_RGB_COLORS) || !(dibparams = wget_DIB_params( // this returns pointers and values, but allocates no memory dib, &px, &ct, &numCt, &width, &height, &colortype, &invert )) ){
Now if the first condition is true ("(iUsage != U_DIB_RGB_COLORS)") the second part will not be executed,
That is intentional. What is the full message? Does it go away if this change is made to line 1395?
int dibparams;to
int dibparams = U_BI_RGB;
The full warning I don't have anymore. It's about uninitialized variables. There are many variables that are not initialized immediately in that function which is very bad by itself and should be fixed. In particular, not initializing dibparams is a bug because of the if-statement later on: if(dibparams == U_BI_JPEG .... Regardless if it's a real bug or not, please immediately initialize every variable always, unless it is extremely obvious that it's going to be initialized correctly in the next 2 lines.
Please also add a comment that the ordering in the if-statement is intentional, because that is not obvious at all from the code. (please also think about avoiding relying on the short-circuit evaluation)
Thanks for the fixes, Johan
Patch entered into launchpad here:
https://bugs.launchpad.net/inkscape/+bug/1248753
Johan, please try it and see if it resolves the clang issue.
Regards,
David Mathog mathog@...1176... Manager, Sequence Analysis Facility, Biology Division, Caltech
participants (2)
-
Johan Engelen -
mathog