Re: [PATCH 1/3] msvcrt: Don't set errno when calling trigonometric functions with NAN.

[Piotr Caban]

Hi,

I've done some more reading about it and it looks like our 
implementation is not behaving in similar way as native does. The error 
handling should call _matherr function (see also __setusermatherr).

Thanks to implementing it that way we easily write code that is 
compatible with older versions of C-runtime (by providing different 
default implementation of matherr function that prevents setting errno). 
I think it's best to implement it that way to avoid potential regressions.

Thanks,
Piotr

[PATCH 1/3] msvcrt: Don't set errno when calling trigonometric functions with NAN.

[Alex Henrie]

2017-07-21 9:49 GMT-06:00 Piotr Caban :
> Hi,
>
> I've done some more reading about it and it looks like our implementation is
> not behaving in similar way as native does. The error handling should call
> _matherr function (see also __setusermatherr).
>
> Thanks to implementing it that way we easily write code that is compatible
> with older versions of C-runtime (by providing different default
> implementation of matherr function that prevents setting errno). I think
> it's best to implement it that way to avoid potential regressions.

In order to move the error checks to the user matherr function, the
user matherr function would have to be called whenever any version of
MSVC would set errno. Then, each version of matherr would have to
suppress the error if the function name and arguments are within a
certain set. This isn't right: For example, testing shows that in
msvcr120, sin(NAN) calls the user matherr function, but in ucrtbase it
doesn't. If a program that uses ucrtbase replaces the default user
matherr function, the custom user matherr function would be called
when it should not.

The right way to set errno differently based on the MSVC version is to
create separate, versioned implementations of each function.

The errno behavior is not relevant to the bugs I was trying to fix; it
was just something that I thought I could fix quickly and easily. Do
you think that it is worthwhile for me to continue working on it?

-Alex

[PATCH 1/3] msvcrt: Don't set errno when calling trigonometric functions with NAN.

[Piotr Caban]

On 07/24/17 07:40, Alex Henrie wrote:
> The errno behavior is not relevant to the bugs I was trying to fix; it
> was just something that I thought I could fix quickly and easily. Do
> you think that it is worthwhile for me to continue working on it?

I think that there's more value in fixing bugs that lets application 
run. On the other hand this code can use some cleanup. I'm just not sure 
how to review code that doesn't fix any real application, changes 
function behavior to be compatible with newer version of the dll while 
making it less compatible with older versions. I was hoping that there 
might be a solution that will preserve compatibility with older versions 
and improve compatibility with newer ones (not necessarily setting 
errno/calling _matherr in the same way as native does for all versions 
of the dll).

If you're going to change most of this functions it's worth adding 
proper support for matherr function.

Thanks,
Piotr