From 5396d8a1ab52b4da4f5199109472fe7f8ea43085 Mon Sep 17 00:00:00 2001 From: nobu Date: Wed, 23 Mar 2016 11:57:01 +0000 Subject: [PATCH] strftime.c: format in String * strftime.c (rb_strftime_with_timespec): append formatted results to the given string with expanding, and also deal with NUL chars. * strftime.c (rb_strftime, rb_strftime_timespec): return formatted string, not the length put in the given buffer. * time.c (rb_strftime_alloc): no longer needs to retry with reallocating buffers. * time.c (time_strftime): no longer needs to split by NUL chars. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@54236 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 13 ++++ common.mk | 1 + internal.h | 8 +-- strftime.c | 147 ++++++++++++++++++++++++++--------------- test/ruby/test_time.rb | 3 +- time.c | 87 +++++------------------- 6 files changed, 129 insertions(+), 130 deletions(-) diff --git a/ChangeLog b/ChangeLog index 91252c4a2e..bf56c10b06 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +Wed Mar 23 20:56:59 2016 Nobuyoshi Nakada + + * strftime.c (rb_strftime_with_timespec): append formatted results + to the given string with expanding, and also deal with NUL chars. + + * strftime.c (rb_strftime, rb_strftime_timespec): return formatted + string, not the length put in the given buffer. + + * time.c (rb_strftime_alloc): no longer needs to retry with + reallocating buffers. + + * time.c (time_strftime): no longer needs to split by NUL chars. + Wed Mar 23 14:23:54 2016 NARUSE, Yui * lib/rdoc/ri/driver.rb (interactive): rescue NotFoundError raised in diff --git a/common.mk b/common.mk index cc1d98eea8..39616a73b5 100644 --- a/common.mk +++ b/common.mk @@ -2254,6 +2254,7 @@ strftime.$(OBJEXT): {$(VPATH)}config.h strftime.$(OBJEXT): {$(VPATH)}defines.h strftime.$(OBJEXT): {$(VPATH)}encoding.h strftime.$(OBJEXT): {$(VPATH)}intern.h +strftime.$(OBJEXT): {$(VPATH)}internal.h strftime.$(OBJEXT): {$(VPATH)}missing.h strftime.$(OBJEXT): {$(VPATH)}oniguruma.h strftime.$(OBJEXT): {$(VPATH)}st.h diff --git a/internal.h b/internal.h index 38175d776e..e5a098db5f 100644 --- a/internal.h +++ b/internal.h @@ -1214,10 +1214,10 @@ int rb_sigaltstack_size(void); /* strftime.c */ #ifdef RUBY_ENCODING_H -size_t rb_strftime_timespec(char *s, size_t maxsize, const char *format, rb_encoding *enc, - const struct vtm *vtm, struct timespec *ts, int gmt); -size_t rb_strftime(char *s, size_t maxsize, const char *format, rb_encoding *enc, - const struct vtm *vtm, VALUE timev, int gmt); +VALUE rb_strftime_timespec(const char *format, size_t format_len, rb_encoding *enc, + const struct vtm *vtm, struct timespec *ts, int gmt); +VALUE rb_strftime(const char *format, size_t format_len, rb_encoding *enc, + const struct vtm *vtm, VALUE timev, int gmt); #endif /* string.c */ diff --git a/strftime.c b/strftime.c index 83550e9a7f..a426a455bd 100644 --- a/strftime.c +++ b/strftime.c @@ -50,6 +50,7 @@ #include "ruby/ruby.h" #include "ruby/encoding.h" #include "timev.h" +#include "internal.h" #ifndef GAWK #include @@ -156,16 +157,35 @@ max(int a, int b) /* strftime --- produce formatted time */ +static char * +resize_buffer(VALUE ftime, char *s, const char **start, const char **endp, + ptrdiff_t n) +{ + size_t len = s - *start; + size_t nlen = len + n * 2; + rb_str_set_len(ftime, len); + rb_str_modify_expand(ftime, nlen-len); + s = RSTRING_PTR(ftime); + *endp = s + nlen; + *start = s; + return s += len; +} + /* * enc is the encoding of the format. It is used as the encoding of resulted * string, but the name of the month and weekday are always US-ASCII. So it * is only used for the timezone name on Windows. */ -static size_t -rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encoding *enc, const struct vtm *vtm, VALUE timev, struct timespec *ts, int gmt) +static VALUE +rb_strftime_with_timespec(VALUE ftime, const char *format, size_t format_len, + rb_encoding *enc, const struct vtm *vtm, VALUE timev, + struct timespec *ts, int gmt) { - const char *const endp = s + maxsize; - const char *const start = s; + size_t len = RSTRING_LEN(ftime); + char *s = RSTRING_PTR(ftime); + const char *start = s; + const char *endp = start + rb_str_capacity(ftime); + const char *const format_end = format + format_len; const char *sp, *tp; #define TBUFSIZE 100 auto char tbuf[TBUFSIZE]; @@ -193,27 +213,28 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi }; static const char ampm[][3] = { "AM", "PM", }; - if (s == NULL || format == NULL || vtm == NULL || maxsize == 0) - return 0; - - /* quick check if we even need to bother */ - if (strchr(format, '%') == NULL && strlen(format) + 1 >= maxsize) { + if (format == NULL || format_len == 0 || vtm == NULL) { err: - errno = ERANGE; return 0; } - if (enc && (enc == rb_usascii_encoding() || - enc == rb_ascii8bit_encoding() || enc == rb_locale_encoding())) { - enc = NULL; + if (enc && + (enc == rb_usascii_encoding() || + enc == rb_ascii8bit_encoding() || + enc == rb_locale_encoding())) { + enc = NULL; } - for (; *format && s < endp - 1; format++) { + s += len; + for (; format < format_end; format++) { #define FLAG_FOUND() do { \ if (precision > 0) \ goto unknown; \ } while (0) -#define NEEDS(n) do if (s >= endp || (n) >= endp - s - 1) goto err; while (0) +#define NEEDS(n) do { \ + if (s >= endp || (n) >= endp - s - 1) \ + s = resize_buffer(ftime, s, &start, &endp, (n)); \ + } while (0) #define FILL_PADDING(i) do { \ if (!(flags & BIT_OF(LEFT)) && precision > (i)) { \ NEEDS(precision); \ @@ -226,25 +247,34 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi } while (0); #define FMT(def_pad, def_prec, fmt, val) \ do { \ - int l; \ if (precision <= 0) precision = (def_prec); \ if (flags & BIT_OF(LEFT)) precision = 1; \ - l = snprintf(s, endp - s, \ - ((padding == '0' || (!padding && (def_pad) == '0')) ? "%0*"fmt : "%*"fmt), \ - precision, (val)); \ - if (l < 0) goto err; \ - s += l; \ + len = s - start; \ + NEEDS(precision); \ + rb_str_set_len(ftime, len); \ + rb_str_catf(ftime, \ + ((padding == '0' || (!padding && (def_pad) == '0')) ? "%0*"fmt : "%*"fmt), \ + precision, (val)); \ + RSTRING_GETMEM(ftime, s, len); \ + endp = (start = s) + rb_str_capacity(ftime); \ + s += len; \ } while (0) #define STRFTIME(fmt) \ do { \ - i = rb_strftime_with_timespec(s, endp - s, (fmt), enc, vtm, timev, ts, gmt); \ - if (!i) return 0; \ + len = s - start; \ + rb_str_set_len(ftime, len); \ + if (!rb_strftime_with_timespec(ftime, (fmt), rb_strlen_lit(fmt), enc, vtm, timev, ts, gmt)) \ + return 0; \ + s = RSTRING_PTR(ftime); \ + i = RSTRING_LEN(ftime) - len; \ + endp = (start = s) + rb_str_capacity(ftime); \ + s += len; \ if (precision > i) {\ NEEDS(precision); \ memmove(s + precision - i, s, i);\ memset(s, padding ? padding : ' ', precision - i); \ s += precision; \ - }\ + } \ else s += i; \ } while (0) #define FMTV(def_pad, def_prec, fmt, val) \ @@ -271,10 +301,14 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi } \ } while (0) - if (*format != '%') { - *s++ = *format; - continue; - } + tp = memchr(format, '%', format_end - format); + if (!tp) tp = format_end; + NEEDS(tp - format); + memcpy(s, format, tp - format); + s += tp - format; + format = tp; + if (format == format_end) break; + tp = tbuf; sp = format; precision = -1; @@ -282,11 +316,8 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi padding = 0; colons = 0; again: - switch (*++format) { - case '\0': - format--; - goto unknown; - + if (++format >= format_end) goto unknown; + switch (*format) { case '%': FILL_PADDING(1); *s++ = '%'; @@ -768,12 +799,12 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi goto again; case ':': - { - size_t l = strspn(format, ":"); - if (l > 3 || format[l] != 'z') goto unknown; - colons = (int)l; - format += l - 1; + for (colons = 1; colons <= 3; ++colons) { + if (format+colons >= format_end) goto unknown; + if (format[colons] == 'z') break; + if (format[colons] != ':') goto unknown; } + format += colons - 1; goto again; case '0': @@ -781,9 +812,12 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi case '1': case '2': case '3': case '4': case '5': case '6': case '7': case '8': case '9': { - char *e; - precision = (int)strtoul(format, &e, 10); - format = e - 1; + size_t n; + int ov; + unsigned long u = ruby_scan_digits(format, format_end-format, 10, &n, &ov); + if (ov || u > INT_MAX) goto unknown; + precision = (int)u; + format += n - 1; goto again; } @@ -817,26 +851,31 @@ rb_strftime_with_timespec(char *s, size_t maxsize, const char *format, rb_encodi } } } - if (s >= endp) { - goto err; - } - if (*format == '\0') { - *s = '\0'; - return (s - start); - } else + if (s >= endp || format != format_end) { return 0; + } + len = s - start; + rb_str_set_len(ftime, len); + rb_str_resize(ftime, len); + return ftime; } -size_t -rb_strftime(char *s, size_t maxsize, const char *format, rb_encoding *enc, const struct vtm *vtm, VALUE timev, int gmt) +VALUE +rb_strftime(const char *format, size_t format_len, + rb_encoding *enc, const struct vtm *vtm, VALUE timev, int gmt) { - return rb_strftime_with_timespec(s, maxsize, format, enc, vtm, timev, NULL, gmt); + VALUE result = rb_enc_str_new(0, 0, enc); + return rb_strftime_with_timespec(result, format, format_len, enc, + vtm, timev, NULL, gmt); } -size_t -rb_strftime_timespec(char *s, size_t maxsize, const char *format, rb_encoding *enc, const struct vtm *vtm, struct timespec *ts, int gmt) +VALUE +rb_strftime_timespec(const char *format, size_t format_len, + rb_encoding *enc, const struct vtm *vtm, struct timespec *ts, int gmt) { - return rb_strftime_with_timespec(s, maxsize, format, enc, vtm, Qnil, ts, gmt); + VALUE result = rb_enc_str_new(0, 0, enc); + return rb_strftime_with_timespec(result, format, format_len, enc, + vtm, Qnil, ts, gmt); } /* isleap --- is a year a leap year? */ diff --git a/test/ruby/test_time.rb b/test/ruby/test_time.rb index 232d3472c4..71f2756216 100644 --- a/test/ruby/test_time.rb +++ b/test/ruby/test_time.rb @@ -810,8 +810,7 @@ class TestTime < Test::Unit::TestCase end def test_strftime_too_wide - bug4457 = '[ruby-dev:43285]' - assert_raise(Errno::ERANGE, bug4457) {Time.now.strftime('%8192z')} + assert_equal(8192, Time.now.strftime('%8192z').size) end def test_strfimte_zoneoffset diff --git a/time.c b/time.c index ea71e35434..111b71e203 100644 --- a/time.c +++ b/time.c @@ -3536,7 +3536,8 @@ time_get_tm(VALUE time, struct time_object *tobj) return time_localtime(time); } -static VALUE strftimev(const char *fmt, VALUE time, rb_encoding *enc); +static VALUE strftime_cstr(const char *fmt, size_t len, VALUE time, rb_encoding *enc); +#define strftimev(fmt, time, enc) strftime_cstr((fmt), rb_strlen_lit(fmt), (time), (enc)) /* * call-seq: @@ -4202,67 +4203,34 @@ time_to_a(VALUE time) time_zone(time)); } -#define SMALLBUF 100 -static size_t -rb_strftime_alloc(char **buf, VALUE formatv, const char *format, rb_encoding *enc, +static VALUE +rb_strftime_alloc(const char *format, size_t format_len, rb_encoding *enc, struct vtm *vtm, wideval_t timew, int gmt) { - size_t size, len, flen; VALUE timev = Qnil; struct timespec ts; if (!timew2timespec_exact(timew, &ts)) - timev = w2v(rb_time_unmagnify(timew)); + timev = w2v(rb_time_unmagnify(timew)); - (*buf)[0] = '\0'; - flen = strlen(format); - if (flen == 0) { - return 0; + if (NIL_P(timev)) { + return rb_strftime_timespec(format, format_len, enc, vtm, &ts, gmt); } - errno = 0; - if (timev == Qnil) - len = rb_strftime_timespec(*buf, SMALLBUF, format, enc, vtm, &ts, gmt); - else - len = rb_strftime(*buf, SMALLBUF, format, enc, vtm, timev, gmt); - if (len != 0 || (**buf == '\0' && errno != ERANGE)) return len; - for (size=1024; ; size*=2) { - *buf = xmalloc(size); - (*buf)[0] = '\0'; - if (timev == Qnil) - len = rb_strftime_timespec(*buf, size, format, enc, vtm, &ts, gmt); - else - len = rb_strftime(*buf, size, format, enc, vtm, timev, gmt); - /* - * buflen can be zero EITHER because there's not enough - * room in the string, or because the control command - * goes to the empty string. Make a reasonable guess that - * if the buffer is 1024 times bigger than the length of the - * format string, it's not failing for lack of room. - */ - if (len > 0) break; - xfree(*buf); - if (size >= 1024 * flen) { - if (!NIL_P(formatv)) rb_sys_fail_str(formatv); - rb_sys_fail(format); - break; - } + else { + return rb_strftime(format, format_len, enc, vtm, timev, gmt); } - return len; } static VALUE -strftimev(const char *fmt, VALUE time, rb_encoding *enc) +strftime_cstr(const char *fmt, size_t len, VALUE time, rb_encoding *enc) { struct time_object *tobj; - char buffer[SMALLBUF], *buf = buffer; - long len; VALUE str; GetTimeval(time, tobj); MAKE_TM(time, tobj); - len = rb_strftime_alloc(&buf, Qnil, fmt, enc, &tobj->vtm, tobj->timew, TIME_UTC_P(tobj)); - str = rb_enc_str_new(buf, len, enc); - if (buf != buffer) xfree(buf); + str = rb_strftime_alloc(fmt, len, enc, &tobj->vtm, tobj->timew, TIME_UTC_P(tobj)); + if (!str) rb_raise(rb_eArgError, "invalid format: %s", fmt); return str; } @@ -4457,11 +4425,9 @@ static VALUE time_strftime(VALUE time, VALUE format) { struct time_object *tobj; - char buffer[SMALLBUF], *buf = buffer; const char *fmt; long len; rb_encoding *enc; - VALUE str; GetTimeval(time, tobj); MAKE_TM(time, tobj); @@ -4475,33 +4441,14 @@ time_strftime(VALUE time, VALUE format) enc = rb_enc_get(format); if (len == 0) { rb_warning("strftime called with empty format string"); - } - else if (fmt[len] || memchr(fmt, '\0', len)) { - /* Ruby string may contain \0's. */ - const char *p = fmt, *pe = fmt + len; - - str = rb_str_new(0, 0); - while (p < pe) { - len = rb_strftime_alloc(&buf, format, p, enc, - &tobj->vtm, tobj->timew, TIME_UTC_P(tobj)); - rb_str_cat(str, buf, len); - p += strlen(p); - if (buf != buffer) { - xfree(buf); - buf = buffer; - } - for (fmt = p; p < pe && !*p; ++p); - if (p > fmt) rb_str_cat(str, fmt, p - fmt); - } - return str; + return rb_enc_str_new(0, 0, enc); } else { - len = rb_strftime_alloc(&buf, format, RSTRING_PTR(format), enc, - &tobj->vtm, tobj->timew, TIME_UTC_P(tobj)); + VALUE str = rb_strftime_alloc(fmt, len, enc, &tobj->vtm, tobj->timew, + TIME_UTC_P(tobj)); + if (!str) rb_raise(rb_eArgError, "invalid format: %"PRIsVALUE, format); + return str; } - str = rb_enc_str_new(buf, len, enc); - if (buf != buffer) xfree(buf); - return str; } /* :nodoc: */