Cppcheck report in vcl/unx/gtk/gdi/salnativewidgets-gtk.cxx

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

Cppcheck report in vcl/unx/gtk/gdi/salnativewidgets-gtk.cxx

Hello,

Just noticed this with cppcheck:
[gdi/salnativewidgets-gtk.cxx:3452] -> [gdi/salnativewidgets-gtk.cxx:3462]: (performance) Variable 'eState' is reassigned a value before the old one has been used.
Here are the lines:
   3452     GtkStateType eState = (nState & CTRL_STATE_ENABLED) ? GTK_STATE_ACTIVE : GTK_STATE_INSENSITIVE;
   3453     gint slider_width = 10;
   3454     gint slider_length = 10;
   3455     gint trough_border = 0;
   3456     gtk_widget_style_get( pWidget,
   3457                           "slider-width", &slider_width,
   3458                           "slider-length", &slider_length,
   3459                           "trough-border", &trough_border,
   3460                           NULL);
   3461
   3462     eState = (nState & CTRL_STATE_ENABLED) ? GTK_STATE_NORMAL : GTK_STATE_INSENSITIVE;
   3463     if( nPart == PART_TRACK_HORZ_AREA )
   3464     {
   3465         gtk_paint_box( pWidget->style,
   3466                        pixDrawable,
   3467                        eState,
   3468                        GTK_SHADOW_IN,
   3469                        NULL,
   3470                        pWidget,
   3471                        "trough",
   3472                        0, (h-slider_width-2*trough_border)/2, w, slider_width + 2*trough_border);

Is the line 3452 could just be removed or should this part be a little reworked?

Julien
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck report in vcl/unx/gtk/gdi/salnativewidgets-gtk.cxx

On 20.10.2012 22:23, julien2412 wrote:

> Hello,
>
> Just noticed this with cppcheck:
> [gdi/salnativewidgets-gtk.cxx:3452] -> [gdi/salnativewidgets-gtk.cxx:3462]:
> (performance) Variable 'eState' is reassigned a value before the old one has
> been used.
> Here are the lines:
>    3452     GtkStateType eState = (nState & CTRL_STATE_ENABLED) ?
> GTK_STATE_ACTIVE : GTK_STATE_INSENSITIVE;
>    3453     gint slider_width = 10;
>    3454     gint slider_length = 10;
>    3455     gint trough_border = 0;
>    3456     gtk_widget_style_get( pWidget,
>    3457                           "slider-width", &slider_width,
>    3458                           "slider-length", &slider_length,
>    3459                           "trough-border", &trough_border,
>    3460                           NULL);
>    3461
>    3462     eState = (nState & CTRL_STATE_ENABLED) ? GTK_STATE_NORMAL :
> GTK_STATE_INSENSITIVE;
>    3463     if( nPart == PART_TRACK_HORZ_AREA )
>    3464     {
>    3465         gtk_paint_box( pWidget->style,
>    3466                        pixDrawable,
>    3467                        eState,
>    3468                        GTK_SHADOW_IN,
>    3469                        NULL,
>    3470                        pWidget,
>    3471                        "trough",
>    3472                        0, (h-slider_width-2*trough_border)/2, w,
> slider_width + 2*trough_border);
>
> Is the line 3452 could just be removed or should this part be a little
> reworked?

Hi Julien,

I think the line 3452 can be removed. GTK_STATE_ACTIVE is for "pressed"
controls, i.e. CTRL_STATE_PRESSED, and the Slider control does not even
use this CTRL_STATE_PRESSED flag.
http://opengrok.libreoffice.org/xref/core/vcl/source/control/slider.cxx

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

Re: Cppcheck report in vcl/unx/gtk/gdi/salnativewidgets-gtk.cxx

Ivan Timofeev wrote
On 20.10.2012 22:23, julien2412 wrote:
> [gdi/salnativewidgets-gtk.cxx:3452] -> [gdi/salnativewidgets-gtk.cxx:3462]:
> (performance) Variable 'eState' is reassigned a value before the old one has
> been used.
> Is the line 3452 could just be removed or should this part be a little
> reworked?
I think the line 3452 can be removed. GTK_STATE_ACTIVE is for "pressed"
controls, i.e. CTRL_STATE_PRESSED, and the Slider control does not even
use this CTRL_STATE_PRESSED flag.
http://opengrok.libreoffice.org/xref/core/vcl/source/control/slider.cxx
Hi Ivan,

Thank you for your feedback Ivan, I pushed this removal on master (see http://cgit.freedesktop.org/libreoffice/core/commit/?id=b339e4e2ba2b833903956cbcebf64a2a99dba176)

Julien