Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(351)

Issue 5502069: code review 5502069: exp/norm: fixed two unrelated bugs in normalization library. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 5 months ago by mpvl
Modified:
13 years, 5 months ago
Reviewers:
CC:
r, golang-dev
Visibility:
Public.

Description

exp/norm: fixed two unrelated bugs in normalization library. 1) incorrect length given for out buffer in String. 2) patchTail bug that could cause characters to be lost when crossing into the out-buffer boundary. Added tests to expose these bugs. Also slightly improved performance of Bytes() and String() by sharing the reorderBuffer across operations. Fixes issue 2567.

Patch Set 1 #

Patch Set 2 : diff -r 8259f4a19e6e https://go.googlecode.com/hg #

Patch Set 3 : diff -r 8259f4a19e6e https://go.googlecode.com/hg #

Total comments: 1

Patch Set 4 : diff -r d0d549cc5e45 https://go.googlecode.com/hg #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -30 lines) Patch
M src/pkg/exp/norm/input.go View 3 chunks +3 lines, -5 lines 0 comments Download
M src/pkg/exp/norm/normalize.go View 1 2 3 6 chunks +31 lines, -22 lines 0 comments Download
M src/pkg/exp/norm/normalize_test.go View 3 chunks +16 lines, -1 line 0 comments Download
M src/pkg/exp/norm/readwriter.go View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 4
mpvl
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg
13 years, 5 months ago (2011-12-22 19:53:48 UTC) #1
mpvl
Hello r@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
13 years, 5 months ago (2011-12-22 19:55:31 UTC) #2
r
LGTM http://codereview.appspot.com/5502069/diff/4001/src/pkg/exp/norm/normalize.go File src/pkg/exp/norm/normalize.go (right): http://codereview.appspot.com/5502069/diff/4001/src/pkg/exp/norm/normalize.go#newcode177 src/pkg/exp/norm/normalize.go:177: buf, endsinerror := patchTail(rb, out) endsInError
13 years, 5 months ago (2011-12-22 21:02:38 UTC) #3
mpvl
13 years, 5 months ago (2011-12-23 17:21:32 UTC) #4
*** Submitted as http://code.google.com/p/go/source/detail?r=cb41d013cf7f ***

exp/norm: fixed two unrelated bugs in normalization library.
1) incorrect length given for out buffer in String.
2) patchTail bug that could cause characters to be lost
   when crossing into the out-buffer boundary.

Added tests to expose these bugs.  Also slightly improved
performance of Bytes() and String() by sharing the reorderBuffer
across operations.

Fixes issue 2567.

R=r
CC=golang-dev
http://codereview.appspot.com/5502069
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b