yyang42 / moulitest

This repository contains tests for several projects done at 42.
127 stars 37 forks source link

Two corrections for ft_printf tests #84

Closed gavinfielder closed 5 years ago

gavinfielder commented 5 years ago

I discovered two errors:

In suite 93, the test was specifying %lf to test long double, when the size modifier is actually %Lf. The mismatch was resulting in 80-bit long double data being read as 64-bit double data, which probably has undefined behavior.

In suite 52, the 0 flag produces undefined behavior when used with %p. This is something that should have been caught in the last update.

yyang42 commented 5 years ago

Are we using C89, C99 or C11 at 42 ?

I would suggest:

If you're stuck with a C89 library, "%lf" is undefined; in C99 and C11 libraries it is defined to be the same as "%f". – pmg Apr 29 '12 at 0:06

https://stackoverflow.com/questions/4264127/correct-format-specifier-for-double-in-printf#comment13362944_4264127

gavinfielder commented 5 years ago

Are we using C89, C99 or C11 at 42 ?

Honestly it's an open question. Some subjects suggest that using C99 is against the norme but every indication from the compilers on the Fremont campus say C99 is the one we use. The printf(3) manpage on the Fremont campus says %lf is identical to %f, but since that's connected to the standard in use, I suspect that's something that could easily change between campuses.

gavinfielder commented 5 years ago

One thing to note is that the subject requires %lf to be handled. Based on that, I think adding a separate test for it would be good.

yyang42 commented 5 years ago

In that case, it would be better to support both %lf and %Lf, even if %lf is undefined for C89. With something like (same for other cases).

    assert_printf("{%f}{%lf}{%Lf}", 1.42, 1.42, 1.42l);
gavinfielder commented 5 years ago

Sure, I'll add that tomorrow then.

gavinfielder commented 5 years ago

The previous commit (ab719bfeb2ea9c32ee22a65805096110fd20ed84) adds {%f}{%lf}{%Lf}

yyang42 commented 5 years ago

Awesome, thx!!