Re: [Qemu-devel] [PULL 18/22] fpu/softfloat: re-factor int/uint to float

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 21 February 2018 at 11:05, Alex Bennée <alex.bennee@linaro.org> wrote:
>> +/*
>> + * Integer to float conversions
>> + *
>> + * Returns the result of converting the two's complement integer `a'
>> + * to the floating-point format. The conversion is performed according
>> + * to the IEC/IEEE Standard for Binary Floating-Point Arithmetic.
>> + */
>> +
>> +static FloatParts int_to_float(int64_t a, float_status *status)
>> +{
>> +    FloatParts r;
>> +    if (a == 0) {
>> +        r.cls = float_class_zero;
>> +        r.sign = false;
>> +    } else if (a == (1ULL << 63)) {
>> +        r.cls = float_class_normal;
>> +        r.sign = true;
>> +        r.frac = DECOMPOSED_IMPLICIT_BIT;
>> +        r.exp = 63;
>> +    } else {
>> +        uint64_t f;
>> +        if (a < 0) {
>> +            f = -a;
>> +            r.sign = true;
>> +        } else {
>> +            f = a;
>> +            r.sign = false;
>> +        }
>> +        int shift = clz64(f) - 1;
>> +        r.cls = float_class_normal;
>> +        r.exp = (DECOMPOSED_BINARY_POINT - shift);
>> +        r.frac = f << shift;
>> +    }
>> +
>> +    return r;
>> +}
>
> Hi -- Coverity complains about this function (CID1390635) because
> there's a code path through it that doesn't fully initialize
> the struct (the a == 0 case doesn't set r.frac), and it thinks
> that "return r" is a 'use' of all fields in the structure.
> I don't know why it doesn't complain about r.exp.
>
> Should we initialize all of r's fields anyway to shut it up,
> or mark it as a Coverity false-positive?

Hmm tricky - because in some cases we don't want to mess with it. The
fcvt patch for example manually messed with the frac portion to deal
with conversion. But it's done outside of the main canonicalize/pack
routines.

>
> thanks
> -- PMM


--
Alex Bennée