Re: [virt-viewer][PATCH 2/2] coverity: result is not floating-point

[Fabiano Fidencio <ffidenci@xxxxxxxxxx>]

On Mon, Aug 17, 2015 at 6:08 PM, Pavel Grunt <pgrunt@xxxxxxxxxx> wrote:

On Mon, 2015-08-17 at 17:55 +0200, Fabiano Fidêncio wrote:
> Coverity says:
> Result is not floating-point (UNINTENDED_INTEGER_DIVISION)
> interger_division: Dividing integer expressions "preferred->width * 100"
> and "zoom", and then converting the integer quotient to type double. Any
> remainder, or fractional part of the quotient, is ignored.

I think it is better to remove the round(), otherwise you are changing the
behavior (which is there since 33614f86db490364339ef69e0eb76f98a4ac8138).

I don't see why I am (wrongly) changing the behavior, Pavel.

Can you give me an example?

Nowadays, using the round or not using the round would end up in the same result and I don't think it's the expected/correct behavior.

Let's consider: width = 640, NORMAL_ZOOM_LEVEL = 100, zoom = 85.

Current behavior:

width = round (640*100/85)
width = round (752)
width = 752.

Removing the round:

width = 640*100/85
width = 752

What I consider the expected behavior:
width = round (640*100/85)
width = round (752.94)
width = 753

 

Or is the rounding necessary ?

I do think so.

 

Pavel

> ---
>  src/virt-viewer-display.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c
> index 3efe24c..8431ae4 100644
> --- a/src/virt-viewer-display.c
> +++ b/src/virt-viewer-display.c
> @@ -819,8 +819,8 @@ void
> virt_viewer_display_get_preferred_monitor_geometry(VirtViewerDisplay* self,
>      if (virt_viewer_display_get_zoom(VIRT_VIEWER_DISPLAY(self))) {
>          guint zoom =
> virt_viewer_display_get_zoom_level(VIRT_VIEWER_DISPLAY(self));
>
> -        preferred->width = round(preferred->width * NORMAL_ZOOM_LEVEL /
> zoom);
> -        preferred->height = round(preferred->height * NORMAL_ZOOM_LEVEL /
> zoom);
> +        preferred->width = round(preferred->width * NORMAL_ZOOM_LEVEL /
> (double) zoom);
> +        preferred->height = round(preferred->height * NORMAL_ZOOM_LEVEL /
> (double) zoom);
>      }
>  }
>

Thanks for the review,

--

Fabiano Fidêncio

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list