code review 3095042: syscall: remove terminating \r and \n from windows erro...

syscall: remove terminating \r and \n from windows error messages

[brainman, 2010-11-16 02:39:09]

Hello golang-dev@googlegroups.com,

I'd like you to review this change.

[peterGo, 2010-11-16 06:51:08]

Is this patch is going to work? Also, is it going to panic if n is ever zero?

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
File src/pkg/syscall/syscall_windows.go (left):

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:163: }
Inserting this code should work.

// trim any newline
for ; n > 0 && (b[n-1] == '\n' || b[n-1] == '\r'); n-- {
}

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
File src/pkg/syscall/syscall_windows.go (right):

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:160: n, err := FormatMessage(flags, 0,
uint32(errno), 0, b, nil)
n is the number of characters, excluding the terminating null character.

Return Value. If the function succeeds, the return value is the number of TCHARs
stored in the output buffer, excluding the terminating null character.
FormatMessage Function
http://msdn.microsoft.com/en-us/library/ms679351.aspx

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:164: // remove terminating 0
This isn't true. n (a uint32) is the number of characters, excluding the
terminating null character; n is being changed to an index into the array of
characters.

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:172: return string(utf16.Decode(b[:n]))
Now n is back to being the number of characters. However, since n is an array
index value, it's off by one.

[brainman, 2010-11-17 00:19:04]

Hello golang-dev@googlegroups.com, PeterGo (cc: golang-dev@googlegroups.com,
rsc),

Please take another look.

[brainman, 2010-11-17 00:19:04]

Thank you for review. You're right, I'm wrong.

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
File src/pkg/syscall/syscall_windows.go (left):

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:163: }
On 2010/11/16 06:51:08, PeterGo wrote:
> Inserting this code should work.
> 
> // trim any newline
> for ; n > 0 && (b[n-1] == '\n' || b[n-1] == '\r'); n-- {
> }

Done.

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
File src/pkg/syscall/syscall_windows.go (right):

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:164: // remove terminating 0
On 2010/11/16 06:51:08, PeterGo wrote:
> This isn't true. n (a uint32) is the number of characters, excluding the
> terminating null character; n is being changed to an index into the array of
> characters.

Done.

http://codereview.appspot.com/3095042/diff/2001/src/pkg/syscall/syscall_windo...
src/pkg/syscall/syscall_windows.go:172: return string(utf16.Decode(b[:n]))
On 2010/11/16 06:51:08, PeterGo wrote:
> Now n is back to being the number of characters. However, since n is an array
> index value, it's off by one.

Done.

[peterGo, 2010-11-17 00:51:33]

LGTM

[rsc1, 2010-12-09 19:07:04]

LGTM

[brainman, 2010-12-13 06:02:09]

*** Submitted as http://code.google.com/p/go/source/detail?r=26e97d57efe8 ***

syscall: remove terminating \r and \n from windows error messages

R=rsc, peterGo
CC=golang-dev
http://codereview.appspot.com/3095042

Committer: Alex Brainman <alex.brainman@gmail.com>