code review 9437044: os: use small buffer when reading from windows console

os: use small buffer when reading from windows console

Fixes issue 5481.

[brainman, 2013-05-16 02:49:27]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[Dominik Honnef, 2013-05-16 03:03:41]

https://codereview.appspot.com/9437044/diff/5001/src/pkg/os/file_windows.go
File src/pkg/os/file_windows.go (right):

https://codereview.appspot.com/9437044/diff/5001/src/pkg/os/file_windows.go#n...
src/pkg/os/file_windows.go:269: println(len(wchars), err.Error())
Left-over debug statement?

[brainman, 2013-05-16 03:15:26]

Hello golang-dev@googlegroups.com, dominik.honnef@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[brainman, 2013-05-16 03:15:32]

Thank you for review.

Alex

https://codereview.appspot.com/9437044/diff/5001/src/pkg/os/file_windows.go
File src/pkg/os/file_windows.go (right):

https://codereview.appspot.com/9437044/diff/5001/src/pkg/os/file_windows.go#n...
src/pkg/os/file_windows.go:269: println(len(wchars), err.Error())
On 2013/05/16 03:03:41, Dominik Honnef wrote:
> Left-over debug statement?

Sure thing. Those pesky buggers. Unfortunately there is no test to catch them.

Removed it.

[bradfitz, 2013-05-16 06:50:55]

LGTM



On Wed, May 15, 2013 at 7:49 PM, <alex.brainman@gmail.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> os: use small buffer when reading from windows console
>
> Fixes issue 5481.
>
> Please review this at
https://codereview.appspot.**com/9437044/<https://codereview.appspot.com/9437...
>
> Affected files:
>   M src/pkg/os/file_windows.go
>
>
> Index: src/pkg/os/file_windows.go
> ==============================**==============================**=======
> --- a/src/pkg/os/file_windows.go
> +++ b/src/pkg/os/file_windows.go
> @@ -251,8 +251,14 @@
>                 return 0, nil
>         }
>         if len(f.readbuf) == 0 {
> +               // syscall.ReadConsole seems to fail, if given large
> buffer.
> +               // So limit the buffer to 16000 characters.
> +               readN := 16000
> +               if len(b) < readN {
> +                       readN = len(b)
> +               }
>                 // get more input data from os
> -               wchars := make([]uint16, len(b))
> +               wchars := make([]uint16, readN)
>                 var p *uint16
>                 if len(b) > 0 {
>                         p = &wchars[0]
> @@ -260,6 +266,7 @@
>                 var nw uint32
>                 err := syscall.ReadConsole(f.fd, p, uint32(len(wchars)),
> &nw, nil)
>                 if err != nil {
> +                       println(len(wchars), err.Error())
>                         return 0, err
>                 }
>                 f.readbuf = utf16.Decode(wchars[:nw])
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[brainman, 2013-05-16 07:20:21]

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

os: use small buffer when reading from windows console

Fixes issue 5481.

R=golang-dev, dominik.honnef, bradfitz
CC=golang-dev
https://codereview.appspot.com/9437044

[r, 2013-05-16 14:15:11]

https://codereview.appspot.com/9437044/diff/17001/src/pkg/os/file_windows.go
File src/pkg/os/file_windows.go (right):

https://codereview.appspot.com/9437044/diff/17001/src/pkg/os/file_windows.go#...
src/pkg/os/file_windows.go:256: readN := 16000
this seems backwards relative to the comment. also readN makes sense but is a
little unusual. about

numBytes := len(b)
if numBytes > 16000 {
   numBytes = 16000
}

[brainman, 2013-05-17 02:56:29]

On 2013/05/16 14:15:11, r wrote:
> this seems backwards relative to the comment...

Agreed. Here https://codereview.appspot.com/9458043/ is the change.

Alex