code review 109020043: bufio: handle excessive whitespace in ScanWords

bufio: handle excessive whitespace in ScanWords

[mdempsky, 2014-06-16 06:30:49]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2014-06-16 16:39:01]

R=r



On Sun, Jun 15, 2014 at 11:30 PM, mdempsky via golang-codereviews <
golang-codereviews@googlegroups.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> bufio: handle excessive whitespace in ScanWords
>
> Please review this at https://codereview.appspot.com/109020043/
>
> Affected files (+17, -5 lines):
>   M src/pkg/bufio/scan.go
>   M src/pkg/bufio/scan_test.go
>
>
> Index: src/pkg/bufio/scan.go
> ===================================================================
> --- a/src/pkg/bufio/scan.go
> +++ b/src/pkg/bufio/scan.go
> @@ -326,9 +326,6 @@
>                         break
>                 }
>         }
> -       if atEOF && len(data) == 0 {
> -               return 0, nil, nil
> -       }
>         // Scan until space, marking end of word.
>         for width, i := 0, start; i < len(data); i += width {
>                 var r rune
> @@ -342,5 +339,5 @@
>                 return len(data), data[start:], nil
>         }
>         // Request more data.
> -       return 0, nil, nil
> +       return start, nil, nil
>  }
> Index: src/pkg/bufio/scan_test.go
> ===================================================================
> --- a/src/pkg/bufio/scan_test.go
> +++ b/src/pkg/bufio/scan_test.go
> @@ -15,6 +15,8 @@
>         "unicode/utf8"
>  )
>
> +const smallMaxTokenSize = 256 // Much smaller for more efficient testing.
> +
>  // Test white space table matches the Unicode definition.
>  func TestSpace(t *testing.T) {
>         for r := rune(0); r <= utf8.MaxRune; r++ {
> @@ -172,7 +174,6 @@
>
>  // Test the line splitter, including some carriage returns but no long
> lines.
>  func TestScanLongLines(t *testing.T) {
> -       const smallMaxTokenSize = 256 // Much smaller for more efficient
> testing.
>         // Build a buffer of lots of line lengths up to but not exceeding
> smallMaxTokenSize.
>         tmp := new(bytes.Buffer)
>         buf := new(bytes.Buffer)
> @@ -404,3 +405,17 @@
>                 t.Errorf("unexpected error: %v", err)
>         }
>  }
> +
> +func TestScanWordsExcessiveWhiteSpace(t *testing.T) {
> +       const word = "ipsum"
> +       s := strings.Repeat(" ", 4*smallMaxTokenSize) + word
> +       scanner := NewScanner(strings.NewReader(s))
> +       scanner.MaxTokenSize(smallMaxTokenSize)
> +       scanner.Split(ScanWords)
> +       if !scanner.Scan() {
> +               t.Fatal("scan failed: %v", scanner.Err())
> +       }
> +       if token := scanner.Text(); token != word {
> +               t.Fatal("unexpected token: %v", token)
> +       }
> +}
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[r, 2014-06-16 19:56:01]

LGTM

[r, 2014-06-16 19:58:17]

*** Submitted as https://code.google.com/p/go/source/detail?r=f866ee1cef9d ***

bufio: handle excessive white space in ScanWords

LGTM=r
R=golang-codereviews, bradfitz, r
CC=golang-codereviews
https://codereview.appspot.com/109020043

Committer: Rob Pike <r@golang.org>

[gobot, 2014-06-16 20:11:30]

This CL appears to have broken the netbsd-386-minux builder.
See http://build.golang.org/log/60a73a58297ff9be523dd22957f1ebcdc1864faf

[minux, 2014-06-16 20:24:53]

On Mon, Jun 16, 2014 at 4:11 PM, <gobot@golang.org> wrote:

> This CL appears to have broken the netbsd-386-minux builder.
> See http://build.golang.org/log/60a73a58297ff9be523dd22957f1ebcdc1864faf

Unrelated, but this is the first time this test failed IIRC. Possibly a
flaky test?

# GOMAXPROCS=2 runtime -cpu=1,2,4
--- FAIL: TestEmptyString-4 (4.27 seconds)
mfinal_test.go:213: finalizer of next string in memory didn't run
FAIL
FAIL runtime 99.444s
Build complete, duration 6m31.74797264s. Result: error: exit status 1

[mdempsky, 2014-08-06 02:31:21]

*** Abandoned ***