|
|
|
Created:
11 years, 1 month ago by dvyukov Modified:
11 years ago Reviewers:
CC:
golang-codereviews, bradfitz, minux Visibility:
Public. |
runtime: fix error handling in file operations on darwin
Currently it's impossible to understand whether a file operation
succeeds or not, because functions always return positive values.
Discovered while working on the tracing functionality.
The traced ended up not needing it, but I think it's still worth fixing.
Patch Set 1 #Patch Set 2 : diff -r d053a9fe2d0153e961e3767dcc6896eeec658af7 https://dvyukov%40google.com@code.google.com/p/go/ #Patch Set 3 : diff -r d053a9fe2d0153e961e3767dcc6896eeec658af7 https://dvyukov%40google.com@code.google.com/p/go/ #Patch Set 4 : diff -r d053a9fe2d0153e961e3767dcc6896eeec658af7 https://dvyukov%40google.com@code.google.com/p/go/ #Patch Set 5 : diff -r d053a9fe2d0153e961e3767dcc6896eeec658af7 https://dvyukov%40google.com@code.google.com/p/go/ #Total messages: 6
|
dvyukov
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to the dev.cc branch of https://dvyukov%40google.com@code.google.com/p/go/
|
11 years, 1 month ago (2014年11月21日 17:45:25 UTC) #1 |
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to the dev.cc branch of https://dvyukov%40google.com@code.google.com/p/go/
After Go 1.4, of course. But you might want to file a bug so this isn't lost in the hg->git move. Well, we'll probably want to manually go over https://go-dev.appspot.com/#all and make sure we move all pending CLs there to git, or nag the authors to. On Fri, Nov 21, 2014 at 9:45 AM, dvyukov 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 the dev.cc branch of > https://dvyukov%40google.com@code.google.com/p/go/ > > > Description: > runtime: fix error handling in file operations on darwin > Currently it's impossible to understand whether a file operation > succeeds or not, because functions always return positive values. > Discovered while working on the tracing functionality. > The traced ended up not needing it, but I think it's still worth fixing. > > Please review this at https://codereview.appspot.com/179130043/ > > Affected files (+9, -0 lines): > M src/runtime/sys_darwin_amd64.s > > > Index: src/runtime/sys_darwin_amd64.s > =================================================================== > --- a/src/runtime/sys_darwin_amd64.s > +++ b/src/runtime/sys_darwin_amd64.s > @@ -38,6 +38,9 @@ > MOVL perm+12(FP), DX // arg 3 mode > MOVL $(0x2000000+5), AX // syscall entry > SYSCALL > + JCC ok > + NEGQ AX > +ok: > MOVL AX, ret+16(FP) > RET > > @@ -54,6 +57,9 @@ > MOVL n+16(FP), DX // arg 3 count > MOVL $(0x2000000+3), AX // syscall entry > SYSCALL > + JCC ok > + NEGQ AX > +ok: > MOVL AX, ret+24(FP) > RET > > @@ -63,6 +69,9 @@ > MOVL n+16(FP), DX // arg 3 count > MOVL $(0x2000000+4), AX // syscall entry > SYSCALL > + JCC ok > + NEGQ AX > +ok: > MOVL AX, ret+24(FP) > RET > > > > -- > 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. >
This change is for dev.cc branch, the branch is not frozen for release. On Fri, Nov 21, 2014 at 9:38 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > After Go 1.4, of course. But you might want to file a bug so this isn't lost > in the hg->git move. > > Well, we'll probably want to manually go over > https://go-dev.appspot.com/#all and make sure we move all pending CLs there > to git, or nag the authors to. > > > On Fri, Nov 21, 2014 at 9:45 AM, dvyukov 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 the dev.cc branch of >> https://dvyukov%40google.com@code.google.com/p/go/ >> >> >> Description: >> runtime: fix error handling in file operations on darwin >> Currently it's impossible to understand whether a file operation >> succeeds or not, because functions always return positive values. >> Discovered while working on the tracing functionality. >> The traced ended up not needing it, but I think it's still worth fixing. >> >> Please review this at https://codereview.appspot.com/179130043/ >> >> Affected files (+9, -0 lines): >> M src/runtime/sys_darwin_amd64.s >> >> >> Index: src/runtime/sys_darwin_amd64.s >> =================================================================== >> --- a/src/runtime/sys_darwin_amd64.s >> +++ b/src/runtime/sys_darwin_amd64.s >> @@ -38,6 +38,9 @@ >> MOVL perm+12(FP), DX // arg 3 mode >> MOVL $(0x2000000+5), AX // syscall entry >> SYSCALL >> + JCC ok >> + NEGQ AX >> +ok: >> MOVL AX, ret+16(FP) >> RET >> >> @@ -54,6 +57,9 @@ >> MOVL n+16(FP), DX // arg 3 count >> MOVL $(0x2000000+3), AX // syscall entry >> SYSCALL >> + JCC ok >> + NEGQ AX >> +ok: >> MOVL AX, ret+24(FP) >> RET >> >> @@ -63,6 +69,9 @@ >> MOVL n+16(FP), DX // arg 3 count >> MOVL $(0x2000000+4), AX // syscall entry >> SYSCALL >> + JCC ok >> + NEGQ AX >> +ok: >> MOVL AX, ret+24(FP) >> RET >> >> >> >> >> -- >> 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. > >
In general, all the functions in sys_$GOOS_$GOARCH.s only checks for error if their callers need to. (None of the runtime.open function checks for error, for example.) And as the existing callers don't check for errors and the tracing functionality also don't need that, I don't think we need to add these checks.
Oh, I missed that before, and I was even looking for it. I guess I'm already getting confused by seeing a new code review template coming in (for Gerrit). On Fri, Nov 21, 2014 at 11:02 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > This change is for dev.cc branch, the branch is not frozen for release. > > On Fri, Nov 21, 2014 at 9:38 PM, Brad Fitzpatrick <bradfitz@golang.org> > wrote: > > After Go 1.4, of course. But you might want to file a bug so this isn't > lost > > in the hg->git move. > > > > Well, we'll probably want to manually go over > > https://go-dev.appspot.com/#all and make sure we move all pending CLs > there > > to git, or nag the authors to. > > > > > > On Fri, Nov 21, 2014 at 9:45 AM, dvyukov 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 the dev.cc branch of > >> https://dvyukov%40google.com@code.google.com/p/go/ > >> > >> > >> Description: > >> runtime: fix error handling in file operations on darwin > >> Currently it's impossible to understand whether a file operation > >> succeeds or not, because functions always return positive values. > >> Discovered while working on the tracing functionality. > >> The traced ended up not needing it, but I think it's still worth fixing. > >> > >> Please review this at https://codereview.appspot.com/179130043/ > >> > >> Affected files (+9, -0 lines): > >> M src/runtime/sys_darwin_amd64.s > >> > >> > >> Index: src/runtime/sys_darwin_amd64.s > >> =================================================================== > >> --- a/src/runtime/sys_darwin_amd64.s > >> +++ b/src/runtime/sys_darwin_amd64.s > >> @@ -38,6 +38,9 @@ > >> MOVL perm+12(FP), DX // arg 3 mode > >> MOVL $(0x2000000+5), AX // syscall entry > >> SYSCALL > >> + JCC ok > >> + NEGQ AX > >> +ok: > >> MOVL AX, ret+16(FP) > >> RET > >> > >> @@ -54,6 +57,9 @@ > >> MOVL n+16(FP), DX // arg 3 count > >> MOVL $(0x2000000+3), AX // syscall entry > >> SYSCALL > >> + JCC ok > >> + NEGQ AX > >> +ok: > >> MOVL AX, ret+24(FP) > >> RET > >> > >> @@ -63,6 +69,9 @@ > >> MOVL n+16(FP), DX // arg 3 count > >> MOVL $(0x2000000+4), AX // syscall entry > >> SYSCALL > >> + JCC ok > >> + NEGQ AX > >> +ok: > >> MOVL AX, ret+24(FP) > >> RET > >> > >> > >> > >> > >> -- > >> 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=close To the author of this CL: The Go project has moved to Gerrit Code Review. If this CL should be continued, please see the latest version of https://golang.org/doc/contribute.html for instructions on how to set up Git and the Go project's Gerrit codereview plugin, and then create a new change with your current code. If there has been discussion on this CL, please give a link to it (golang.org/cl/179130043 is best) in the description in your new CL. Thanks very much.