This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Replace use of snprintf with strfrom in libm tests
- From: Joseph Myers <joseph at codesourcery dot com>
- To: "Gabriel F. T. Gomes" <gftg at linux dot vnet dot ibm dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Tue, 13 Dec 2016 18:56:38 +0000
- Subject: Re: [PATCH] Replace use of snprintf with strfrom in libm tests
- Authentication-results: sourceware.org; auth=none
- References: <1481563942-19034-1-git-send-email-gftg@linux.vnet.ibm.com>
On Mon, 12 Dec 2016, Gabriel F. T. Gomes wrote:
> In order to support float128 tests, the calls to snprintf, which does
> not support the type __float128, are replaced with calls to
> strfrom{f,d,l}.
This patch appears to lose the effects of the ' ' flag, i.e. aligning
positive and negative values in the output.
I think that alignment is desirable, meaning you need another level of
wrappers to handle moving the output of FTOSTR and prepending it with a
space in the case where a '-' was not output. This is relevant both to
the cases where you're inserting a precision in a format string, and to
those where the precision was already embedded in the string.
Also, the PRINTF_EXPR, PRINTF_XEXPR, PRINTF_NEXPR macros only made sense
at all in a context where snprintf was used. With the use of strfrom,
they don't depend on the type used, and the code would be clearer if it
just used "e", "a", "f" directly in libm-test.inc (with the macro
definitions and the comments about them in libm-test.inc being removed).
(Actually I think there are just four cases involved, which would indicate
further refactoring: "% .*e" with TYPE_DECIMAL_DIG - 1 precision, "% .*a"
with TYPE_HEX_DIG - 1 precision, "%.0f" and "% .4f". The third of these
should work unchanged with strfrom. The fourth is only used with
nonnegative values, so a space could be inserted in the users instead of
needing special handling to emulate the ' ' flag. But I don't think such
further refactoring is needed for this change to go in, as long as you
keep output unchanged and eliminate type-specific definitions of macros
PRINTF_* when all types have the same definitions.)
> +/* The definitions TYPE_DECIMAL_DIG and TYPE_HEX_DIG are used to select the
> + precision (i.e.: number of fractional digits) to be printed. When using
> + snprintf, it is possible to pass the precision in an argument with "%.*".
> + On the other hand, strfrom does not accept such format string, thus the
> + precision must be coded in the format string itself. */
> +static int
> +fmt_ftostr (char *dest, size_t size, const char *format,
> + const char *conversion, int precision, FLOAT value)
The function comment should describe what the function does, including the
semantics of its arguments and return value.
> + /* Generate the format string. */
> + ptr_format = stpcpy(new_format, format);
> + ret = sprintf(ptr_format, "%d", precision);
> + ptr_format += ret;
> + ptr_format = stpcpy(ptr_format, conversion);
> +
> + /* Call the float to string conversion function, e.g.: strfromd. */
> + return FTOSTR(dest, size, new_format, value);
Missing spaces before '(', several times.
> - FTOSTR (fstrn, FSTR_MAX, "% .*" PRINTF_EXPR, TYPE_DECIMAL_DIG - 1, f);
> - FTOSTR (fstrx, FSTR_MAX, "% .*" PRINTF_XEXPR, TYPE_HEX_DIG - 1, f);
> + fmt_ftostr (fstrn, FSTR_MAX, "%.", PRINTF_EXPR, TYPE_DECIMAL_DIG - 1, f);
> + fmt_ftostr (fstrx, FSTR_MAX, "%.", PRINTF_XEXPR, TYPE_HEX_DIG - 1, f);
These are examples of cases where I think the space-padding for
non-negative results should be preserved (possibly by making fmt_ftostr do
that padding, if it's only ever used in cases where ' ' was used before).
You could e.g. compare test-ldouble.out (as an example of nontrivial
output on powerpc) before and after the changes to make sure there aren't
unexpected changes to the output of the tests.
--
Joseph S. Myers
joseph@codesourcery.com