Cppcheck reports 'else if' condition matches previous condition (vcl)

classic Classic list List threaded Threaded
3 messages Options
julien2412 julien2412
Reply | Threaded
Open this post in threaded view
|

Cppcheck reports 'else if' condition matches previous condition (vcl)

Hello,

Cppcheck reported this:
vcl/source/gdi/impimage.cxx
300 multiCondition style Expression is always false because 'else if' condition matches previous condition at line 298.

Indeed we have:
    296                 else
    297                 {
    298                     if( aTmpBmpEx.IsAlpha() )
    299                         aTmpBmpEx = BitmapEx( aTmpBmp, aTmpBmpEx.GetAlpha() );
    300                     else if( aTmpBmpEx.IsAlpha() )
    301                         aTmpBmpEx = BitmapEx( aTmpBmp, aTmpBmpEx.GetMask() );
    302                 }
see http://opengrok.libreoffice.org/xref/core/vcl/source/gdi/impimage.cxx#298

Should the else if be:
else if( aTmpBmpEx.IsTransparent())
?

Julien
Caolán McNamara Caolán McNamara
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck reports 'else if' condition matches previous condition (vcl)

On Sat, 2014-04-05 at 16:06 -0700, julien2412 wrote:

> Indeed we have:
>     296                 else
>     297                 {
>     298                     if( aTmpBmpEx.IsAlpha() )
>     299                         aTmpBmpEx = BitmapEx( aTmpBmp,
> aTmpBmpEx.GetAlpha() );
>     300                     else if( aTmpBmpEx.IsAlpha() )
>     301                         aTmpBmpEx = BitmapEx( aTmpBmp,
> aTmpBmpEx.GetMask() );
>     302                 }
> see
> http://opengrok.libreoffice.org/xref/core/vcl/source/gdi/impimage.cxx#298
>
> Should the else if be:
> else if( aTmpBmpEx.IsTransparent())

"grep -r IsAlpha -C 5 vcl|grep else" definitely strongly suggests that
was the intent. I reckon its worth making that change and see if
anything falls over. Not the kind of thing I'd backport to 4-2 unless we
get compelling evidence to do that.

C.

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
julien2412 julien2412
Reply | Threaded
Open this post in threaded view
|

[SOLVED] Re: Cppcheck reports 'else if' condition matches previous condition (vcl)

On 11/04/2014 16:13, Caolán McNamara wrote:

> On Sat, 2014-04-05 at 16:06 -0700, julien2412 wrote:
>> Indeed we have:
>>      296                 else
>>      297                 {
>>      298                     if( aTmpBmpEx.IsAlpha() )
>>      299                         aTmpBmpEx = BitmapEx( aTmpBmp,
>> aTmpBmpEx.GetAlpha() );
>>      300                     else if( aTmpBmpEx.IsAlpha() )
>>      301                         aTmpBmpEx = BitmapEx( aTmpBmp,
>> aTmpBmpEx.GetMask() );
>>      302                 }
>> see
>> http://opengrok.libreoffice.org/xref/core/vcl/source/gdi/impimage.cxx#298
>>
>> Should the else if be:
>> else if( aTmpBmpEx.IsTransparent())
> "grep -r IsAlpha -C 5 vcl|grep else" definitely strongly suggests that
> was the intent. I reckon its worth making that change and see if
> anything falls over. Not the kind of thing I'd backport to 4-2 unless we
> get compelling evidence to do that.
Thank you Caolán, I pushed the change in master, see
http://cgit.freedesktop.org/libreoffice/core/commit/?id=6c6f78c6567db73aa85de4beb461db2cddac7b2a

Julien

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice