FAQ
Reviewers: golang-dev_googlegroups.com,

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

I'd like you to review this change to
https://code.google.com/p/go/


Description:
net/rpc: give hint to pass a pointer to Register
Fixes issue 4325.

Please review this at http://codereview.appspot.com/6819081/

Affected files:
M src/pkg/net/rpc/server.go
M src/pkg/net/rpc/server_test.go


Index: src/pkg/net/rpc/server.go
===================================================================
--- a/src/pkg/net/rpc/server.go
+++ b/src/pkg/net/rpc/server.go
@@ -261,8 +261,30 @@
s.method = make(map[string]*methodType)

// Install the methods
- for m := 0; m < s.typ.NumMethod(); m++ {
- method := s.typ.Method(m)
+ s.method = suitableMethods(s.typ, true)
+
+ if len(s.method) == 0 {
+ str := ""
+ // for convenience, we check if a pointer to the current type is correct
+ method := suitableMethods(reflect.PtrTo(s.typ), false)
+ if len(method) != 0 {
+ str = "rpc Register: type " + sname + " has no exported methods of
suitable type (hint: pass a pointer to it)"
+ } else {
+ str = "rpc Register: type " + sname + " has no exported methods of
suitable type"
+ }
+ log.Print(str)
+ return errors.New(str)
+ }
+ server.serviceMap[s.name] = s
+ return nil
+}
+
+// suitableMethods returns suitable Rpc methods of typ, it will report
+// error using log if reportErr is true.
+func suitableMethods(typ reflect.Type, reportErr bool)
map[string]*methodType {
+ methods := make(map[string]*methodType)
+ for m := 0; m < typ.NumMethod(); m++ {
+ method := typ.Method(m)
mtype := method.Type
mname := method.Name
// Method must be exported.
@@ -271,46 +293,51 @@
}
// Method needs three ins: receiver, *args, *reply.
if mtype.NumIn() != 3 {
- log.Println("method", mname, "has wrong number of ins:", mtype.NumIn())
+ if reportErr {
+ log.Println("method", mname, "has wrong number of ins:", mtype.NumIn())
+ }
continue
}
// First arg need not be a pointer.
argType := mtype.In(1)
if !isExportedOrBuiltinType(argType) {
- log.Println(mname, "argument type not exported:", argType)
+ if reportErr {
+ log.Println(mname, "argument type not exported:", argType)
+ }
continue
}
// Second arg must be a pointer.
replyType := mtype.In(2)
if replyType.Kind() != reflect.Ptr {
- log.Println("method", mname, "reply type not a pointer:", replyType)
+ if reportErr {
+ log.Println("method", mname, "reply type not a pointer:", replyType)
+ }
continue
}
// Reply type must be exported.
if !isExportedOrBuiltinType(replyType) {
- log.Println("method", mname, "reply type not exported:", replyType)
+ if reportErr {
+ log.Println("method", mname, "reply type not exported:", replyType)
+ }
continue
}
// Method needs one out.
if mtype.NumOut() != 1 {
- log.Println("method", mname, "has wrong number of outs:",
mtype.NumOut())
+ if reportErr {
+ log.Println("method", mname, "has wrong number of outs:",
mtype.NumOut())
+ }
continue
}
// The return type of the method must be error.
if returnType := mtype.Out(0); returnType != typeOfError {
- log.Println("method", mname, "returns", returnType.String(), "not
error")
+ if reportErr {
+ log.Println("method", mname, "returns", returnType.String(), "not
error")
+ }
continue
}
- s.method[mname] = &methodType{method: method, ArgType: argType,
ReplyType: replyType}
+ methods[mname] = &methodType{method: method, ArgType: argType,
ReplyType: replyType}
}
-
- if len(s.method) == 0 {
- s := "rpc Register: type " + sname + " has no exported methods of
suitable type"
- log.Print(s)
- return errors.New(s)
- }
- server.serviceMap[s.name] = s
- return nil
+ return methods
}

// A value sent as a placeholder for the server's response value when the
server
Index: src/pkg/net/rpc/server_test.go
===================================================================
--- a/src/pkg/net/rpc/server_test.go
+++ b/src/pkg/net/rpc/server_test.go
@@ -349,6 +349,7 @@
type ReplyNotPointer int
type ArgNotPublic int
type ReplyNotPublic int
+type NeedsPtrType int
type local struct{}

func (t *ReplyNotPointer) ReplyNotPointer(args *Args, reply Reply) error {
@@ -363,6 +364,10 @@
return nil
}

+func (t *NeedsPtrType) NeedsPtrType(args *Args, reply *Reply) error {
+ return nil
+}
+
// Check that registration handles lots of bad methods and a type with no
suitable methods.
func TestRegistrationError(t *testing.T) {
err := Register(new(ReplyNotPointer))
@@ -377,6 +382,12 @@
if err == nil {
t.Errorf("expected error registering ReplyNotPublic")
}
+ err = Register(NeedsPtrType(0))
+ if err == nil {
+ t.Errorf("expected error registering NeedsPtrType")
+ } else if !strings.Contains(err.Error(), "pointer") {
+ t.Errorf("expected hint when registering NeedsPtrType")
+ }
}

type WriteFailCodec int

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 3, '12 at 7:38a
activeNov 6, '12 at 9:03p
posts3
users2
websitegolang.org

2 users in discussion

Minux Ma: 2 posts R: 1 post

People

Translate

site design / logo © 2022 Grokbase