[Spice-devel] [spice-common 7/8] quic: Remove test which is always TRUE

[Uri Lublin]

On 01/06/2014 01:22 PM, Christophe Fergeau wrote:
> find_model_params() is first dereferencing nbuckets when setting
> it to 0, and then it checks it for NULL. The NULL-check will never trigger
> as if the pointer was NULL, we'd crash when we dereference it.
> This commit removes the redundant test, but adds an assert so that we
> catch this condition if it ever happens.
> ---
>   common/quic.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/common/quic.c b/common/quic.c
> index 2cffde5..cef05ae 100644
> --- a/common/quic.c
> +++ b/common/quic.c
> @@ -954,11 +954,13 @@ static void find_model_params(Encoder *encoder,
>       bsize = *firstsize;
>   
>       do { /* other buckets */

Hi Christophe,

Maybe the condition was supposed to be
    if (*nbuckets)

The logic is -- for the first bucket start from 0,
                       for non-first buckets start where last bucket's end.

Thanks,
     Uri.

> -        if (nbuckets) {         /* bucket start */
> -            bstart = bend + 1;
> -        } else {
> -            bstart = 0;
> -        }
> +        /* There used to be some additional code when nbuckets is NULL, but
> +         * since we dereference nbuckets a few lines before this loop, we
> +         * would have crashed before reaching this test. Adding an assert
> +         * for good measure, but this should never trigger */
> +        spice_assert(nbuckets != NULL);
> +
> +        bstart =  bend + 1;
>   
>           if (!--repcntr) {         /* bucket size */
>               repcntr = *repnext;