-
Notifications
You must be signed in to change notification settings - Fork 43
Conversation
// about outstanding requests. | ||
tags++ | ||
fcall := newFcall(tags, req.message) | ||
var selected Tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind compartmentalizing this into a method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Summary of changes:
|
@@ -107,11 +107,6 @@ type Fcall struct { | |||
} | |||
|
|||
func newFcall(tag Tag, msg Message) *Fcall { | |||
switch msg.Type() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check that this doesn't break negotiation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clientnegotiate and servernegotiate both hardcode NOTAG to the call. Negotiation to my rather pedantic server also works with 9pr.
@Joushou One check on the tag for negiotiation process and we're good! |
I have removed that check, and added commentary that the map must not contain NOTAG (it will otherwise give an erroneous tag pool depletion error when there is still an available tag). |
LGTM @justincormack PTAL |
LGTM |
@Joushou Thanks for the great contribution! Before merging, we'll need you to sign your commits. (Sorry, normally, this is automated, but we have yet to set this up for go-p9p) |
All 4 commits signed. |
New tag allocation system that avoids tag collisions and incorrect use of NOTAG. Signed-off-by: Kenny Levinsen <w@kl.wtf>
Defaulting the tag in newFcall might result in unexpected behaviour. For example, if a client (erroneously) sends a Tversion on a tag, the server response will not be on a tag. newFcall should just create the struct. Signed-off-by: Kenny Levinsen <w@kl.wtf>
Signed-off-by: Kenny Levinsen <w@kl.wtf>
Signed-off-by: Kenny Levinsen <w@kl.wtf>
@Joushou Cheers! |
New tag allocation logic that avoids tag collisions, and errors out if a tag cannot be allocated.