Re: [PATCH 3/4] msvcr120: Add fesetexceptflag.

[Piotr Caban]

Hi Daniel,

On 3/2/21 5:06 AM, Daniel Lehman wrote:
> -
>    except = p_fetestexcept(tests2[i]);
> -
>    ok(except == tests2[i], "expected %lx, got %lx\n", tests2[i], except);
> - }
> -
> - except = FE_ALL_EXCEPT;
> - ret = p_fesetexceptflag(&except, 0);
> - ok(!ret, "fesetexceptflag returned %x\n", ret);
> - except = p_fetestexcept(FE_ALL_EXCEPT);
> - ok(except == FE_ALL_EXCEPT, "expected %x, got %lx\n", FE_ALL_EXCEPT, except);

There's no fpclear call before this test, it probably doesn't test what 
you wanted.
> +/*********************************************************************
> -
> -
>  fesetexceptflag (MSVCR120.@)
> - */
>
> +int CDECL fesetexceptflag(const fexcept_t *status, int excepts)
> +{
> - fenv_t env;
> -
> - if(!status)
> -
>    return 0;

You're adding a test that shows status == NULL crashes on Windows.
> - if(!*status) {
> -
>    _clearfp();
> -
>    return 0;
> - }
> -
> - fegetenv(&env);
> - env._Fe_stat |= (*status & excepts & FE_ALL_EXCEPT);
> - return fesetenv(&env);

I would expect the function to set all the fields specified by mask. 
Probably it should look like:
excepts &= FE_ALL_EXCEPT;
*status &= excepts;
env._Fe_stat = (env._Fe_stat & ~excepts) | *status;
Could you please add some tests that starts with non 0 status word? Also 
please test status on function exit, probably it should be set to new 
status word value.

Thanks,
Piotr