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

[Pavel Grunt]

On Tue, 2015-08-18 at 01:41 +0200, Fabiano Fidencio wrote:
> 
> 
> On Mon, Aug 17, 2015 at 6:08 PM, Pavel Grunt <pgrunt at redhat.com> 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).

Hey Fabiano,
I just suggested a way how to fix the coverity warning without changing the
behavior.
> I don't see why I am (wrongly) changing the behavior, Pavel.
> Can you give me an example?

you gave the example, sometimes there will be an extra pixel, so I was afraid it
can trigger some extra size allocation events. See
 virt_viewer_display_size_allocate() in virt-viewer-display.c and 
virt_viewer_display_spice_size_allocate() virt-viewer-display-spice.c and
virt_viewer_session_on_monitor_geometry_changed() in virt-viewer-session.c

virt_viewer_session_on_monitor_geometry_changed() will be called if
 virt_viewer_display_get_preferred_size() != GtkAllocation, and the requested
size will be set using virt_viewer_display_get_preferred_monitor_geometry()

ACK if you test it and it is ok (no extra size request). It is also possible
that you are avoiding the extra size request.

> 
> 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.
>  
Thanks, it wasn't clear for me that it is about fixing some problem (is there a
"black bar"? - it should be with your example).
 
Pavel

> >  
> > 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