Funnel peerAdd and peerDelete in a channel #1861
Conversation
fcrisciani
force-pushed
the
waitgrp_channel
branch
4 times, most recently
from
213758a to
4b78d93
Compare
common/caller.go
Outdated
|
|
||
| func callerInfo(i int) (string, string, int, bool) { | ||
| ptr, file, line, ok := runtime.Caller(i) | ||
| var funName string |
There was a problem hiding this comment.
nit : "fun"Name ? :) .... is fName good enough ?
There was a problem hiding this comment.
ok, no fun in production code :P
common/caller_test.go
Outdated
| funName, _, _, ok = fun5() | ||
| if !ok || funName != "fun5" { | ||
| t.Fatalf("error on fun5 caller %t, %s", ok, funName) | ||
| } |
There was a problem hiding this comment.
These are certainly fun tests.. hence am okay for the fun%d function names ;)
fcrisciani
force-pushed
the
waitgrp_channel
branch
2 times, most recently
from
110607b to
70fa7f2
Compare
|
Addressed the review comments, submitted the new patch, but let's wait to merge, I will test it locally to be safe, will post here once I've done that |
|
@mavenugo ok tested the following way:
The result is PASS, grep also for error messages from the peer go routine, did not find any |
common/caller.go
Outdated
|
|
||
| // CallerName returns the name of the function at the specified level with additional information | ||
| // level == 0 means current method name | ||
| func CallerName(level int) (string, string, int, bool) { |
There was a problem hiding this comment.
can this just return the function name. I don't think we will be using file and line number at all. We can use this directly in other debugs we have. I have closed the context PR #1835 because we can use this instead.
| @@ -0,0 +1,49 @@ | |||
| package common | |||
There was a problem hiding this comment.
IMO I don't think we need tests for this because runtime.Caller() and runtime.FuncForPC() is part of runtime package. I don't think we should test the runtime package in libnetwork library.
There was a problem hiding this comment.
CallerName is not coming from the runtime, but is simply using some of the runtime functions, so I should be verified that the method results respects the method description
Remove the need for the wait group and avoid new locks Added utility to print the method name and the caller name Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
fcrisciani
force-pushed
the
waitgrp_channel
branch
from
70fa7f2 to
99290ab
Compare
Remove the need for the wait group and avoid new
locks
Signed-off-by: Flavio Crisciani flavio.crisciani@docker.com