[Spice-devel] [usbredir v2] usbredirhost: Fix coverity sign_extension warning

[Victor Toso]

Hi,

On Thu, Aug 04, 2016 at 08:18:19AM -0400, Frediano Ziglio wrote:
> > 
> > Complain from coverity:
> >  usbredir-0.7.1/usbredirhost/usbredirhost.c:1159: sign_extension:
> >  Suspicious implicit sign extension: "max_packetsize" with type
> >  "unsigned short" (16 bits, unsigned) is promoted in
> >  "pkts_per_transfer * transfer_count * max_packetsize" to type "int"
> >  (32 bits, signed), then sign-extended to type "unsigned long"
> >  (64 bits, unsigned). If "pkts_per_transfer * transfer_count *
> >  max_packetsize" is greater than 0x7FFFFFFF, the upper bits of the
> >  result will all be 1.
> > 
> > Pretty sure that reaching 0x7FFFFFFF is improbable but let's make
> > coverity happy.
> > ---
> >  usbredirhost/usbredirhost.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> > index 3e80677..909ebdd 100644
> > --- a/usbredirhost/usbredirhost.c
> > +++ b/usbredirhost/usbredirhost.c
> > @@ -1156,7 +1156,8 @@ static void usbredirhost_stop_stream(struct
> > usbredirhost *host,
> >  static void usbredirhost_set_iso_threshold(struct usbredirhost *host,
> >      uint8_t pkts_per_transfer, uint8_t transfer_count, uint16_t
> >      max_packetsize)
> >  {
> > -    uint64_t reference = pkts_per_transfer * transfer_count *
> > max_packetsize;
> > +    uint64_t reference = ((size_t) (pkts_per_transfer)) * transfer_count *
> > max_packetsize;
> > +
> >      host->iso_threshold.lower = reference / 2;
> >      host->iso_threshold.higher = reference * 3;
> >      DEBUG("higher threshold is %" PRIu64 " bytes | lower threshold is %"
> >      PRIu64 " bytes",
>
> I didn't manage to find the repository for this code.

https://cgit.freedesktop.org/spice/usbredir/tree/usbredirhost/usbredirhost.c#n1156

> Why reference is 64 bit?
> pkts_per_transfer * transfer_count * max_packetsize surely fit in a 32 bit.

Yes.

> Using 32 bit would possibly optimize code a bit if somebody is using 32 bit
> (more unlikely for 64 bit) and would make the print formatting easier.
>
> Frediano

This should be called once per device I think, so optimization is not
really a must but I don't really mind to change it.

Cheers,
  toso