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

Frediano Ziglio fziglio at redhat.com
Thu Aug 4 12:18:19 UTC 2016


> 
> 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.
Why reference is 64 bit? 
pkts_per_transfer * transfer_count * max_packetsize surely fit in a 32 bit.
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


More information about the Spice-devel mailing list