[webkit-reviews] review denied: [Bug 68321] [EFL] Use C++ type cast instead of C style type cast : [Attachment 109168] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 68321: [EFL] Use C++ type cast instead of C style type cast
https://bugs.webkit.org/show_bug.cgi?id=68321

Attachment 109168: Patch
https://bugs.webkit.org/attachment.cgi?id=109168&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=109168&action=review


Nice cleanup, but I have some concerns.

> Source/WebKit/efl/ewk/ewk_view.cpp:930
> -	   px = (double)(x + cx) / (w + sd->view.w);
> +	   px = (x + cx) / (w + sd->view.w);
>      else

Are you sure a cast isn't required here to force floating point arithmetic?

> Source/WebKit/efl/ewk/ewk_view.cpp:934
> -	   py = (double)(y + cy) / (h + sd->view.h);
> +	   py = (y + cy) / (h + sd->view.h);

Ditto.

> Source/WebKit/efl/ewk/ewk_view.cpp:1055
> +    WebCore::IntSize available_size =
WebCore::IntSize(available_rect.width(), available_rect.height());
> +    WebCore::ViewportAttributes attributes =
WebCore::computeViewportAttributes(priv->viewport_arguments, desktop_width,
device_rect.width(), device_rect.height(), device_dpi, available_size);

You should just do available_rect.size()

> Source/WebKit/efl/ewk/ewk_view.cpp:1565
> -	   WRN("zoom level is < %f : %f",
(double)priv->settings.zoom_range.min_scale, (double)zoom);
> +	   WRN("zoom level is < %f : %f", priv->settings.zoom_range.min_scale,
zoom);
>	   return EINA_FALSE;
>      }
>      if (zoom > priv->settings.zoom_range.max_scale) {
> -	   WRN("zoom level is > %f : %f",
(double)priv->settings.zoom_range.max_scale, (double)zoom);
> +	   WRN("zoom level is > %f : %f", priv->settings.zoom_range.max_scale,
zoom);

Are these values not floats? If so why not fix the actual format specifiers
instead of just removing the cast?