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

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  <nobu@ruby-lang.org>
+
+	* 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  <naruse@ruby-lang.org>
 
 	* 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 <stdio.h>
@@ -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: */