FAQ
https://codereview.appspot.com/6569068/diff/18001/src/cmd/dist/buildruntime.c
File src/cmd/dist/buildruntime.c (right):

https://codereview.appspot.com/6569068/diff/18001/src/cmd/dist/buildruntime.c#newcode111
src/cmd/dist/buildruntime.c:111: "#define tos_pid 48\n"
Should this be #define tos_pid(r) 48(r) to be like the others?

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c
File src/pkg/runtime/signal_plan9_386.c (right):

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode33
src/pkg/runtime/signal_plan9_386.c:33: int8 *err[NERRMAX];
This is an array of pointers not an array of char.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode39
src/pkg/runtime/signal_plan9_386.c:39: if(s && *s == 0)
I don't understand this. What is going on here?

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode43
src/pkg/runtime/signal_plan9_386.c:43: if(len <= 4 ||
runtime·mcmp((byte*)s, (byte*)"sys:", 4) != 0)
There are too many exits calls in this function. There shouldn't be any.
In the worst case there should be a noted(NDFLT) to let the kernel kill
the proc (or put it in broken state for debugging).

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode57
src/pkg/runtime/signal_plan9_386.c:57: runtime·exits(s);
Here's another.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode67
src/pkg/runtime/signal_plan9_386.c:67: runtime·memmove((void*)err,
(void*)s, len);
Should check len against ERRMAX. Also, when this function returns the
stack space is gone, so this seems dodgy. Probably you need to save into
a per-m field instead. Or throw it away and use sig->name instead.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_amd64.c
File src/pkg/runtime/signal_plan9_amd64.c (right):

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_amd64.c#newcode22
src/pkg/runtime/signal_plan9_amd64.c:22: runtime·printf("r8 %X\n",
u->r8);
Shouldn't all these %X be lined up?
Or even better do ax\t%X bx\t%X etc

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signals_plan9.h
File src/pkg/runtime/signals_plan9.h (right):

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signals_plan9.h#newcode9
src/pkg/runtime/signals_plan9.h:9: T, "sys: trap:",
I think you need some comments here explaining why this is a throw but
the above were panic.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/stack.h
File src/pkg/runtime/stack.h (right):

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/stack.h#newcode58
src/pkg/runtime/stack.h:58: // purposes like signal handling. Used on
Windows because
Used on Window and on Plan 9
because they do not use a separate stack.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/stack.h#newcode64
src/pkg/runtime/stack.h:64: StackSystem = 512,
Where does 512 come from? You need space for the Ureg and the note and
the call frame.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_386.s
File src/pkg/runtime/sys_plan9_386.s (right):

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_386.s#newcode135
src/pkg/runtime/sys_plan9_386.s:135: SUBL $90, SP
90 is a strange choice, especially since it is not a multiple of 4.
It looks like 24 would work.

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_amd64.s
File src/pkg/runtime/sys_plan9_amd64.s (right):

https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_amd64.s#newcode146
src/pkg/runtime/sys_plan9_amd64.s:146: SUBQ $90, SP
Same comment. Looks like you need $40.

https://codereview.appspot.com/6569068/

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 6, '12 at 10:22p
activeOct 6, '12 at 10:22p
posts1
users1
websitegolang.org

1 user in discussion

Rsc: 1 post

People

Translate

site design / logo © 2022 Grokbase