FAQ

On 2012/11/21 03:34:43, bradfitz wrote:
But why is this comment up here? It should be after the b.Buffered() != 0
check, right?
Depends on how you look it. I view this comment as explaining the entire
if block, not just the != 0 block, but I can easily see the opposite.


https://codereview.appspot.com/6842078/diff/5001/src/pkg/bufio/bufio.go#newcode581
src/pkg/bufio/bufio.go:581: if err = b.Flush(); err1 != nil {
this is misleading. if you really want this behavior, you'd put "err =
b.Flush()" inside the if body, not in the initializer. the idiom is you check
the variable you declared in the condition, but here you don't. you
check err1
instead of err.
This is a bug that the tests are not catching. I'm adding tests for this
section now. Thank you for catching this.



https://codereview.appspot.com/6842078/diff/5001/src/pkg/bufio/bufio.go#newcode612
src/pkg/bufio/bufio.go:612: if err == io.EOF || err ==
io.ErrUnexpectedEOF {
why?
I touch on this earlier in the code, should I repeat the comment?




https://codereview.appspot.com/6842078/

Search Discussions

  • Mchaten at Nov 22, 2012 at 2:40 am

    On 2012/11/21 02:01:09, dfc wrote:
    They shouldn't fail on an unmodified version. The modifications make
    it
    so the test readers and writers behave properly after returning an
    err.
    Without those changes io.ReadAll never fails, causing an infinite
    loop.
    Is it possible to add a test for that condition ?
    Can you be more specific on the exact condition to test?

    https://codereview.appspot.com/6842078/
  • Dave Cheney at Nov 22, 2012 at 3:12 am
    Can you add a test that demonstrates your change prevents the infinite loop in ReadAll ?
    On 22/11/2012, at 13:40, mchaten@gmail.com wrote:
    On 2012/11/21 02:01:09, dfc wrote:
    They shouldn't fail on an unmodified version. The modifications make
    it
    so the test readers and writers behave properly after returning an
    err.
    Without those changes io.ReadAll never fails, causing an infinite
    loop.
    Is it possible to add a test for that condition ?
    Can you be more specific on the exact condition to test?

    https://codereview.appspot.com/6842078/
  • Mchaten at Nov 22, 2012 at 3:32 am
    Hello golang-dev@googlegroups.com, dave@cheney.net, bradfitz@golang.org
    (cc: golang-dev@googlegroups.com, nigeltao@golang.org),

    Please take another look.


    http://codereview.appspot.com/6842078/
  • Mchaten at Nov 22, 2012 at 4:00 am
    Hello golang-dev@googlegroups.com, dave@cheney.net, bradfitz@golang.org
    (cc: golang-dev@googlegroups.com, nigeltao@golang.org),

    Please take another look.


    http://codereview.appspot.com/6842078/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 22, '12 at 2:06a
activeNov 22, '12 at 4:00a
posts5
users2
websitegolang.org

2 users in discussion

Mchaten: 4 posts Dave Cheney: 1 post

People

Translate

site design / logo © 2022 Grokbase