From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10820 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] implement strftime GNU extension padding specifiers '_', '-' and '0' Date: Tue, 20 Dec 2016 15:13:39 -0500 Message-ID: <20161220201339.GL1555@brightrain.aerifal.cx> References: <20161122082908.11584-1-timo.teras@iki.fi> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1482264835 17227 195.159.176.226 (20 Dec 2016 20:13:55 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 20 Dec 2016 20:13:55 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-10833-gllmg-musl=m.gmane.org@lists.openwall.com Tue Dec 20 21:13:51 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1cJQnG-0003LV-L8 for gllmg-musl@m.gmane.org; Tue, 20 Dec 2016 21:13:50 +0100 Original-Received: (qmail 23956 invoked by uid 550); 20 Dec 2016 20:13:53 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 23932 invoked from network); 20 Dec 2016 20:13:52 -0000 Content-Disposition: inline In-Reply-To: <20161122082908.11584-1-timo.teras@iki.fi> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:10820 Archived-At: On Tue, Nov 22, 2016 at 10:29:08AM +0200, Timo Teräs wrote: > --- > For review. GNU and most BSDs seem to implement these extensions. >From what I can tell, only FreeBSD does and they call it a GNU extension. OpenBSD does not have them at all, and also lacks the POSIX-mandated width specifiers. Not sure if FreeBSD has those. > This applies them to numbers only. There might be few cases where this > could be applied more, but I think covers most of the uses. Is there any > test suite I could verify this against? > > src/time/strftime.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) I'm not aware of any test suite. Test cases for strftime (standard features especially -- to make sure none of them are broken by changes) would be really nice to have in libc-test. Overall I'm not too excited about adding nonstandard extensions like this unless there's indication that there's some good chance of consensus among implementations. They have the potential to conflict with future standard requirements and there's no reliable way to test for them at build time (unlike nonstd functions where you can at least check if the symbol is present). So the main thing I'd like before we move forward with this is research into whether other implementations support these already, and if not, what their attitudes towards it are. OTOH if the BSDs aren't even going to adopt the POSIX requirements then maybe they're not terribly relevant here. As we discussed on IRC, python documents that these are nonstandard and depend on your system's strftime, so adding them just for python does not seem justified. I'm not saying no to this outright but I think it should have some more thought before deciding one way or the other. Rich > diff --git a/src/time/strftime.c b/src/time/strftime.c > index e103e02..14fe6f5 100644 > --- a/src/time/strftime.c > +++ b/src/time/strftime.c > @@ -48,12 +48,12 @@ static int week_num(const struct tm *tm) > const char *__tm_to_tzname(const struct tm *); > size_t __strftime_l(char *restrict, size_t, const char *restrict, const struct tm *restrict, locale_t); > > -const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm *tm, locale_t loc) > +const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm *tm, locale_t loc, int pad) > { > nl_item item; > long long val; > const char *fmt = "-"; > - int width = 2; > + int width = 2, def_pad = '0'; > > switch (f) { > case 'a': > @@ -79,15 +79,14 @@ const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm * > case 'C': > val = (1900LL+tm->tm_year) / 100; > goto number; > + case 'e': > + def_pad = '_'; > case 'd': > val = tm->tm_mday; > goto number; > case 'D': > fmt = "%m/%d/%y"; > goto recu_strftime; > - case 'e': > - *l = snprintf(*s, sizeof *s, "%2d", tm->tm_mday); > - return *s; > case 'F': > fmt = "%Y-%m-%d"; > goto recu_strftime; > @@ -200,7 +199,12 @@ const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm * > return 0; > } > number: > - *l = snprintf(*s, sizeof *s, "%0*lld", width, val); > + switch (pad ? pad : def_pad) { > + case '-': *l = snprintf(*s, sizeof *s, "%lld", val); break; > + case '_': *l = snprintf(*s, sizeof *s, "%*lld", width, val); break; > + case '0': > + default: *l = snprintf(*s, sizeof *s, "%0*lld", width, val); break; > + } > return *s; > nl_strcat: > fmt = __nl_langinfo_l(item, loc); > @@ -221,7 +225,7 @@ size_t __strftime_l(char *restrict s, size_t n, const char *restrict f, const st > char buf[100]; > char *p; > const char *t; > - int plus; > + int pad, plus; > unsigned long width; > for (l=0; l if (!*f) { > @@ -233,6 +237,8 @@ size_t __strftime_l(char *restrict s, size_t n, const char *restrict f, const st > continue; > } > f++; > + pad = 0; > + if (*f == '-' || *f == '_' || *f == '0') pad = *f++; > if ((plus = (*f == '+'))) f++; > width = strtoul(f, &p, 10); > if (*p == 'C' || *p == 'F' || *p == 'G' || *p == 'Y') { > @@ -242,7 +248,7 @@ size_t __strftime_l(char *restrict s, size_t n, const char *restrict f, const st > } > f = p; > if (*f == 'E' || *f == 'O') f++; > - t = __strftime_fmt_1(&buf, &k, *f, tm, loc); > + t = __strftime_fmt_1(&buf, &k, *f, tm, loc, pad); > if (!t) break; > if (width) { > for (; *t=='+' || *t=='-' || (*t=='0'&&t[1]); t++, k--); > -- > 2.10.2