Re: msvcrt: fix _copysign(1., -0.) error

[Piotr Caban]

On 03/22/15 14:51, Kevin Chan wrote:
> float CDECL MSVCRT__copysignf( float num, float sign )
>   {
> -    /* FIXME: Behaviour for Nan/Inf? */
> -    if (sign < 0.0)
> -        return num < 0.0 ? num : -num;
> -    return num < 0.0 ? -num : num;
> +    /* FIXME: Behaviour for signbit(NAN) is different in Linux and
> +     *        Windows, where Windows gives a zero for -NAN
> +     */
> +    if (signbit(sign))
> +        return signbit(num) ? num : -num;
> +    return signbit(num) ? -num : num;
> It doesn't make sense to add this comment and remove it in next patch. I 
> would also prefer if signbit fallback implementation is changed in this 
> patch.

+/* FIXME: Doesnot work with +-NAN and -0. */
There's a typo in this comment. Also it works for NAN with sign bit set 
to 0.

+#define signbit(x) ((x) < 0 ? 1 : 0)
This can be simplified to:
#define signbit(x) ((x)<0)

msvcrt: fix _copysign(1., -0.) error

[Zheng Chen]

Hi Piotr,

Thank you very much for pointing out the problems.

And about

  float CDECL MSVCRT__copysignf( float num, float sign )
> > {
> > -    /* FIXME: Behaviour for Nan/Inf? */
> > -    if (sign < 0.0)
> > -        return num < 0.0 ? num : -num;
> > -    return num < 0.0 ? -num : num;
> > +    /* FIXME: Behaviour for signbit(NAN) is different in Linux and
> > +     *        Windows, where Windows gives a zero for -NAN
> > +     */
> > +    if (signbit(sign))
> > +        return signbit(num) ? num : -num;
> > +    return signbit(num) ? -num : num;
>
> It doesn't make sense to add this comment and remove it in next patch. I
> would also prefer if signbit fallback implementation is changed in this
> patch.

I removed the comments now because I thought this is already fixed... I've
combined the changes and sent two patches.

Thanks again :)

Regards,
Zheng