From patchwork Thu Aug 4 20:34:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Paul E. Murphy" X-Patchwork-Id: 655956 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3s51sP5FtYz9s8x for ; Fri, 5 Aug 2016 06:35:21 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=p+DNzBPH; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:date :mime-version:in-reply-to:content-type:message-id; q=dns; s= default; b=MZb82Sg6ib51wBsUhKnvMilIncqo32mFI9aEE+r8CpFoILQfKKRrU RbIcPHkxl2VxxNRD8/akv3ePB7NxZ2jfBHrbqqSFnR1ATNfWnxpKfqvSBhA6iUAL RFItX2fpNjb3P76dqEmGiQY2nADmqJzEDiDXRCN9tpGZuJPeUe5b+0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:date :mime-version:in-reply-to:content-type:message-id; s=default; bh=lJ7MZIjhmTcoEQ+QYEF6br9dpiU=; b=p+DNzBPHN5ojSnHq1BhapzJyIPHR bfUIohXMr9Zaw0oqxN3FZwj+LEaUppn/Ceq92YT6i9fuv8BaKXSo5UwahrJtNrPP 7+psC9JfA76qMaFMehfVzgb6j17MNSL67JtQc+OHp53sF5aY/BPx/xPU+R8wnofD C3RqMkvG91HmRKY= Received: (qmail 94803 invoked by alias); 4 Aug 2016 20:35:11 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 94790 invoked by uid 89); 4 Aug 2016 20:35:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=U*murphyp, murphyplinuxvnetibmcom, murphyp@linux.vnet.ibm.com, sk:murphyp X-HELO: mx0a-001b2d01.pphosted.com X-IBM-Helo: d01dlp01.pok.ibm.com X-IBM-MailFrom: murphyp@linux.vnet.ibm.com Subject: Re: [PATCH] Add tst-wcstod-round To: "Carlos O'Donell" , "libc-alpha@sourceware.org" References: <70c6242f-931e-56e7-e719-32d063303f00@linux.vnet.ibm.com> <5ca50ec8-c9bd-a119-6949-5e436dfd1662@redhat.com> Cc: Joseph Myers From: "Paul E. Murphy" Date: Thu, 4 Aug 2016 15:34:53 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <5ca50ec8-c9bd-a119-6949-5e436dfd1662@redhat.com> X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16080420-0040-0000-0000-000000FB716D X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005549; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000178; SDB=6.00740035; UDB=6.00348013; IPR=6.00512628; BA=6.00004644; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012161; XFM=3.00000011; UTC=2016-08-04 20:34:56 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16080420-0041-0000-0000-000004D5D14C Message-Id: <5dffc1ce-75a4-e878-d6e6-467c991b9db9@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-08-04_12:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608040224 On 08/04/2016 11:17 AM, Carlos O'Donell wrote: > On 08/04/2016 10:50 AM, Paul E. Murphy wrote: >> This extends tst-strtod-round with a few trivial changes >> to also test the wide character variants of strto* using >> similar macros to other shared tests. >> >> * stdlib/tst-strtod-round.c: >> (L_): New macro to select string encoding. >> (FNPFX): New macro to select str or wcs prefix. >> (CHAR): New macro to choose wchar_t or char. >> (STRM): New macro to choose printf modifier for above. >> (STRTO): New macro to choose appropriate string -> real >> function. >> (TEST_WIDE): Conditional definition to enable wchar_t >> testing. >> (FNPFXS): "wcs" or "str" dependent on [TEST_WIDE]. >> (STR): Support for above macro. >> (STRX): Likewise. >> >> (TEST): Update with above macros. >> (test): Likewise. >> (GEN_ONE_TEST): Likewise. >> (test_in_one_mode): Likewise. >> >> * wcsmbs/tst-wcstod-round.c: New file. >> >> * wcsmbs/Makefile: (tests): Add tst-wcstod-round >> (tst-wcstod-round): Add libm depencency for fesetround. > > At a high-level the patch is perfect. > > At a implementation level I'd like to see a little more reorg > to avoid macro API typos. > > See below. Otherwise this looks good to me. > >> --- >> stdlib/tst-strtod-round.c | 43 +++++++++++++++++++++++++++++++++---------- >> wcsmbs/Makefile | 3 +++ >> wcsmbs/tst-wcstod-round.c | 3 +++ >> 3 files changed, 39 insertions(+), 10 deletions(-) >> create mode 100644 wcsmbs/tst-wcstod-round.c >> >> diff --git a/stdlib/tst-strtod-round.c b/stdlib/tst-strtod-round.c >> index 509f96a..023d382 100644 >> --- a/stdlib/tst-strtod-round.c >> +++ b/stdlib/tst-strtod-round.c >> @@ -31,9 +31,26 @@ >> >> #include "tst-strtod.h" >> >> +/* Build the test for wide or normal character strings. */ >> +#ifdef TEST_WIDE > > You should always define TEST_WIDE and set it either to 0 or 1, > but instead I suggest removing it and doing what I suggest below. > > You should still read this: > https://sourceware.org/glibc/wiki/Wundef > >> +# define L_(str) L ## str >> +# define FNPFX wcs >> +# define CHAR wchar_t >> +# define STRM "%S" >> +# define snprintf swprintf >> +# define strfromf128 wcsfromf128 > > This above block of defines should live in tst-wcstod-round.c since > that's the code that needs them, like other similar tests. > >> +#else >> +# define L_(str) str >> +# define FNPFX str >> +# define CHAR char >> +# define STRM "%s" > > These are the defaults macro API values and should always bet set. > I suggest moving this file into a generic test source e.g. > tst-strtox-round.c and then have tst-wcstod-round define the above > defaults and include the generic version. > IMO, in this instance, I think adding another file adds more complexity than it prevents. A test should stand on its own. This is only likely to be ever be used twice without another more substantial refactoring. I've respun the patch remove TEST_WIDE, and some float128 stuff which slipped in. From d32273cae8e3008c3dfdfadc9b31bed0885b9ca8 Mon Sep 17 00:00:00 2001 From: "Paul E. Murphy" Date: Thu, 28 Jul 2016 11:14:11 -0500 Subject: [PATCH] Add tst-wcstod-round This extends tst-strtod-round with a few trivial changes to also test the wide character variants of strto* using similar macros to other shared tests. * stdlib/tst-strtod-round.c: (L_): New macro to select string encoding. (FNPFX): New macro to select str or wcs prefix. (CHAR): New macro to choose wchar_t or char. (STRM): New macro to choose printf modifier for above. (STRTO): New macro to choose appropriate string -> real function. (TEST_WIDE): Conditional definition to enable wchar_t testing. (FNPFXS): "wcs" or "str" dependent on [TEST_WIDE]. (STR): Support for above macro. (STRX): Likewise. (TEST): Update with above macros. (test): Likewise. (GEN_ONE_TEST): Likewise. (test_in_one_mode): Likewise. * wcsmbs/tst-wcstod-round.c: New file. * wcsmbs/Makefile: (tests): Add tst-wcstod-round (tst-wcstod-round): Add libm depencency for fesetround. --- stdlib/tst-strtod-round.c | 45 +++++++++++++++++++++++++++++++++++---------- wcsmbs/Makefile | 3 +++ wcsmbs/tst-wcstod-round.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 10 deletions(-) create mode 100644 wcsmbs/tst-wcstod-round.c diff --git a/stdlib/tst-strtod-round.c b/stdlib/tst-strtod-round.c index 509f96a..b4f7ed1 100644 --- a/stdlib/tst-strtod-round.c +++ b/stdlib/tst-strtod-round.c @@ -31,9 +31,28 @@ #include "tst-strtod.h" +/* Declare default character string macros. */ +#ifndef L_ +# define L_(str) str +#endif + +#ifndef FNPFX +# define FNPFX str +#endif + +#ifndef CHAR +# define CHAR char +#endif + +#ifndef STRM +# define STRM "%s" +#endif + #define _CONCAT(a, b) a ## b #define CONCAT(a, b) _CONCAT (a, b) +#define STRTO(x) CONCAT (CONCAT (FNPFX, to), x) + #if LDBL_MANT_DIG == 106 && LDBL_MAX_EXP == 1024 /* This is a stupid hack for IBM long double. This test ignores inexact values for long double due to the limitations of the @@ -110,7 +129,7 @@ ld106x, ld106d, ld106n, ld106z, ld106u, \ ld113x, ld113d, ld113n, ld113z, ld113u) \ { \ - s, \ + L_ (s), \ { XNTRY (fx, dx, ld64ix, ld64mx, ld106x, ld113x) }, \ { \ { ENTRY (fn, dn, ld64in, ld64mn, ld106n, ld113n) }, \ @@ -131,7 +150,7 @@ struct test_results }; struct test { - const char *s; + const CHAR *s; struct test_exactness exact; struct test_results r[4]; }; @@ -139,19 +158,25 @@ struct test { /* Include the generated test data. */ #include "tst-strtod-round-data.h" +#define STRX(x) #x +#define STR(x) STRX (x) +#define FNPFXS STR (FNPFX) + #define GEN_ONE_TEST(FSUF, FTYPE, FTOSTR, FTOSTRM, LSUF, CSUF) \ { \ - FTYPE f = strto ## FSUF (s, NULL); \ + FTYPE f = STRTO (FSUF) (s, NULL); \ if (f != expected->FSUF \ || (copysign ## CSUF) (1.0 ## LSUF, f) \ != (copysign ## CSUF) (1.0 ## LSUF, expected->FSUF)) \ { \ - char efstr[FSTRLENMAX]; \ - char fstr[FSTRLENMAX]; \ - FTOSTR (efstr, FSTRLENMAX, "%" FTOSTRM "a", expected->FSUF); \ - FTOSTR (fstr, FSTRLENMAX, "%" FTOSTRM "a", f); \ - printf ("strto" #FSUF " (%s) returned %s not %s" \ - " (%s)\n", s, fstr, efstr, mode_name); \ + CHAR efstr[FSTRLENMAX]; \ + CHAR fstr[FSTRLENMAX]; \ + FTOSTR (efstr, FSTRLENMAX, L_("%") L_(FTOSTRM) L_("a"), \ + expected->FSUF); \ + FTOSTR (fstr, FSTRLENMAX, L_("%") L_(FTOSTRM) L_("a"), f);\ + printf (FNPFXS "to" #FSUF " (" STRM ") returned " STRM \ + " not " STRM " (%s)\n", \ + s, fstr, efstr, mode_name); \ if (ROUNDING_TESTS (FTYPE, rnd_mode) || exact->FSUF) \ result = 1; \ else \ @@ -160,7 +185,7 @@ struct test { } static int -test_in_one_mode (const char *s, const struct test_results *expected, +test_in_one_mode (const CHAR *s, const struct test_results *expected, const struct test_exactness *exact, const char *mode_name, int rnd_mode) { diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile index 8b599f7..9384a10 100644 --- a/wcsmbs/Makefile +++ b/wcsmbs/Makefile @@ -49,6 +49,7 @@ strop-tests := wcscmp wcsncmp wmemcmp wcslen wcschr wcsrchr wcscpy wcsnlen \ tests := tst-wcstof wcsmbs-tst1 tst-wcsnlen tst-btowc tst-mbrtowc \ tst-wcrtomb tst-wcpncpy tst-mbsrtowcs tst-wchar-h tst-mbrtowc2 \ tst-c16c32-1 wcsatcliff tst-wcstol-locale tst-wcstod-nan-locale \ + tst-wcstod-round \ $(addprefix test-,$(strop-tests)) include ../Rules @@ -68,6 +69,8 @@ $(objpfx)tst-wcstol-locale.out: $(gen-locales) $(objpfx)tst-wcstod-nan-locale.out: $(gen-locales) endif +$(objpfx)tst-wcstod-round: $(libm) + CFLAGS-wcwidth.c = -I../wctype CFLAGS-wcswidth.c = -I../wctype diff --git a/wcsmbs/tst-wcstod-round.c b/wcsmbs/tst-wcstod-round.c new file mode 100644 index 0000000..1562767 --- /dev/null +++ b/wcsmbs/tst-wcstod-round.c @@ -0,0 +1,31 @@ +/* Test for correct rounding of results of wcstod and related functions. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include + +/* Include stdio.h early to avoid issues with the snprintf + redefinition below. */ +#include + +#define L_(str) L ## str +#define FNPFX wcs +#define CHAR wchar_t +#define STRM "%ls" +#define snprintf swprintf + +#include -- 2.4.11