fmt: reuse buffer and add range checks for %c and %q

Use The fmt internal buffer for character formatting instead of
the pp Printer rune decoding buffer.

Uses an uint64 instead of int64 argument to fmt_c and fmt_qc for easier
range checks since no valid runes are represented by negative numbers or
are above 0x10ffff.

Add range checks to fmt_c and fmt_qc to guarantee that a RuneError
character is returned by the functions for any invalid code point
in range uint64. For invalid code points in range utf8.MaxRune
the used utf8 and strconv functions already return a RuneError.

Change-Id: I9772f804dfcd79c3826fa7f6c5ebfbf4b5304a51
Reviewed-on: https://go-review.googlesource.com/20373
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

src/fmt/fmt_test.go

@@ -225,18 +225,38 @@ var fmtTests = []struct {
 	{"%+q", "abc\xffdef", `"abc\xffdef"`},
 	{"%#q", "abc\xffdef", `"abc\xffdef"`},
 	{"%#+q", "abc\xffdef", `"abc\xffdef"`},
-	{"%q", "\U0010ffff", `"\U0010ffff"`}, // Rune is not printable.
+	// Runes that are not printable.
+	{"%q", "\U0010ffff", `"\U0010ffff"`},
 	{"%+q", "\U0010ffff", `"\U0010ffff"`},
 	{"%#q", "\U0010ffff", "`􏿿`"},
 	{"%#+q", "\U0010ffff", "`􏿿`"},
-	{"%q", string(0x110000), `"<22>"`}, // Rune is not valid.
+	// Runes that are not valid.
+	{"%q", string(0x110000), `"<22>"`},
 	{"%+q", string(0x110000), `"\ufffd"`},
 	{"%#q", string(0x110000), "`<60>`"},
 	{"%#+q", string(0x110000), "`<60>`"},
 
+	// characters
+	{"%c", uint('x'), "x"},
+	{"%c", 0xe4, "ä"},
+	{"%c", 0x672c, "本"},
+	{"%c", '日', "日"},
+	{"%.0c", '⌘', "⌘"}, // Specifying precision should have no effect.
+	{"%3c", '⌘', "  ⌘"},
+	{"%-3c", '⌘', "⌘  "},
+	// Runes that are not printable.
+	{"%c", '\U00000e00', "\u0e00"},
+	{"%c", '\U0010ffff', "\U0010ffff"},
+	// Runes that are not valid.
+	{"%c", -1, "<22>"},
+	{"%c", 0xDC80, "<22>"},
+	{"%c", rune(0x110000), "<22>"},
+	{"%c", int64(0xFFFFFFFFF), "<22>"},
+	{"%c", uint64(0xFFFFFFFFF), "<22>"},
+
 	// escaped characters
-	{"%q", 0, `'\x00'`},
+	{"%q", uint(0), `'\x00'`},
-	{"%+q", 0, `'\x00'`},
+	{"%+q", uint(0), `'\x00'`},
 	{"%q", '"', `'"'`},
 	{"%+q", '"', `'"'`},
 	{"%q", '\'', `'\''`},
@@ -250,8 +270,9 @@ var fmtTests = []struct {
 	{"%q", '\n', `'\n'`},
 	{"%+q", '\n', `'\n'`},
 	{"%q", '☺', `'☺'`},
-	{"% q", '☺', `'☺'`}, // The space modifier should have no effect.
 	{"%+q", '☺', `'\u263a'`},
+	{"% q", '☺', `'☺'`},  // The space modifier should have no effect.
+	{"%.0q", '☺', `'☺'`}, // Specifying precision should have no effect.
 	{"%10q", '⌘', `       '⌘'`},
 	{"%+10q", '⌘', `  '\u2318'`},
 	{"%-10q", '⌘', `'⌘'       `},
@@ -260,12 +281,15 @@ var fmtTests = []struct {
 	{"%+010q", '⌘', `00'\u2318'`},
 	{"%-010q", '⌘', `'⌘'       `}, // 0 has no effect when - is present.
 	{"%+-010q", '⌘', `'\u2318'  `},
-	{"%q", '\U00000e00', `'\u0e00'`},             // Rune is not printable.
+	// Runes that are not printable.
-	{"%q", '\U000c2345', `'\U000c2345'`},         // Rune is not printable.
+	{"%q", '\U00000e00', `'\u0e00'`},
-	{"%q", '\U0010ffff', `'\U0010ffff'`},         // Rune is not printable.
+	{"%q", '\U0010ffff', `'\U0010ffff'`},
-	{"%q", rune(0x110000), `%!q(int32=1114112)`}, // Rune is not valid.
+	// Runes that are not valid.
-	{"%q", int64(0x7FFFFFFF), `%!q(int64=2147483647)`},
+	{"%q", int32(-1), "%!q(int32=-1)"},
-	{"%q", uint64(0xFFFFFFFF), `%!q(uint64=4294967295)`},
+	{"%q", 0xDC80, `'<27>'`},
+	{"%q", rune(0x110000), "%!q(int32=1114112)"},
+	{"%q", int64(0xFFFFFFFFF), "%!q(int64=68719476735)"},
+	{"%q", uint64(0xFFFFFFFFF), "%!q(uint64=68719476735)"},
 
 	// width
 	{"%5s", "abc", "  abc"},
@@ -291,8 +315,6 @@ var fmtTests = []struct {
 	{"%.1x", "日本語", "e6"},
 	{"%.1X", []byte("日本語"), "E6"},
 	{"%10.1q", "日本語日本語", `       "日"`},
-	{"%3c", '⌘', "  ⌘"},
-	{"%5q", '\u2026', `  '…'`},
 	{"%10v", nil, "     <nil>"},
 	{"%-10v", nil, "<nil>     "},
 
@@ -472,10 +494,6 @@ var fmtTests = []struct {
 	{"%G", -7.0, "-7"},
 	{"%G", -1e-9, "-1E-09"},
 	{"%G", float32(-1e-9), "-1E-09"},
-	{"%c", 'x', "x"},
-	{"%c", 0xe4, "ä"},
-	{"%c", 0x672c, "本"},
-	{"%c", '日', "日"},
 	{"%20.8d", 1234, "            00001234"},
 	{"%20.8d", -1234, "           -00001234"},
 	{"%20d", 1234, "                1234"},
@@ -967,6 +985,8 @@ var fmtTests = []struct {
 	// Tests to check that not supported verbs generate an error string.
 	{"%☠", nil, "%!☠(<nil>)"},
 	{"%☠", interface{}(nil), "%!☠(<nil>)"},
+	{"%☠", int(0), "%!☠(int=0)"},
+	{"%☠", uint(0), "%!☠(uint=0)"},
 	{"%☠", []byte{0}, "%!☠([]uint8=[0])"},
 	{"%☠", []uint8{0}, "%!☠([]uint8=[0])"},
 	{"%☠", [1]byte{0}, "%!☠([1]uint8=[0])"},

src/fmt/format.go

@@ -394,14 +394,30 @@ func (f *fmt) fmt_q(s string) {
 	}
 }
 
-// fmt_qc formats the integer as a single-quoted, escaped Go character constant.
+// fmt_c formats an integer as a Unicode character.
 // If the character is not valid Unicode, it will print '\ufffd'.
-func (f *fmt) fmt_qc(c int64) {
+func (f *fmt) fmt_c(c uint64) {
+	r := rune(c)
+	if c > utf8.MaxRune {
+		r = utf8.RuneError
+	}
+	buf := f.intbuf[:0]
+	w := utf8.EncodeRune(buf[:utf8.UTFMax], r)
+	f.pad(buf[:w])
+}
+
+// fmt_qc formats an integer as a single-quoted, escaped Go character constant.
+// If the character is not valid Unicode, it will print '\ufffd'.
+func (f *fmt) fmt_qc(c uint64) {
+	r := rune(c)
+	if c > utf8.MaxRune {
+		r = utf8.RuneError
+	}
 	buf := f.intbuf[:0]
 	if f.plus {
-		f.pad(strconv.AppendQuoteRuneToASCII(buf, rune(c)))
+		f.pad(strconv.AppendQuoteRuneToASCII(buf, r))
 	} else {
-		f.pad(strconv.AppendQuoteRune(buf, rune(c)))
+		f.pad(strconv.AppendQuoteRune(buf, r))
 	}
 }
 

src/fmt/print.go

@@ -116,7 +116,6 @@ type pp struct {
 	reordered bool
 	// goodArgNum records whether the most recent reordering directive was valid.
 	goodArgNum bool
-	runeBuf    [utf8.UTFMax]byte
 	fmt        fmt
 }
 
@@ -340,29 +339,19 @@ func (p *pp) fmtBool(v bool, verb rune) {
 	}
 }
 
-// fmtC formats a rune for the 'c' format.
-func (p *pp) fmtC(c int64) {
-	r := rune(c) // Check for overflow.
-	if int64(r) != c {
-		r = utf8.RuneError
-	}
-	w := utf8.EncodeRune(p.runeBuf[0:utf8.UTFMax], r)
-	p.fmt.pad(p.runeBuf[0:w])
-}
-
 func (p *pp) fmtInt64(v int64, verb rune) {
 	switch verb {
 	case 'b':
 		p.fmt.integer(v, 2, signed, ldigits)
 	case 'c':
-		p.fmtC(v)
+		p.fmt.fmt_c(uint64(v))
 	case 'd', 'v':
 		p.fmt.integer(v, 10, signed, ldigits)
 	case 'o':
 		p.fmt.integer(v, 8, signed, ldigits)
 	case 'q':
 		if 0 <= v && v <= utf8.MaxRune {
-			p.fmt.fmt_qc(v)
+			p.fmt.fmt_qc(uint64(v))
 		} else {
 			p.badVerb(verb)
 		}
@@ -413,7 +402,7 @@ func (p *pp) fmtUint64(v uint64, verb rune) {
 	case 'b':
 		p.fmt.integer(int64(v), 2, unsigned, ldigits)
 	case 'c':
-		p.fmtC(int64(v))
+		p.fmt.fmt_c(v)
 	case 'd':
 		p.fmt.integer(int64(v), 10, unsigned, ldigits)
 	case 'v':
@@ -426,7 +415,7 @@ func (p *pp) fmtUint64(v uint64, verb rune) {
 		p.fmt.integer(int64(v), 8, unsigned, ldigits)
 	case 'q':
 		if 0 <= v && v <= utf8.MaxRune {
-			p.fmt.fmt_qc(int64(v))
+			p.fmt.fmt_qc(v)
 		} else {
 			p.badVerb(verb)
 		}