Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: ensure WriteTo on Windows sends even zero-byte payloads #27446

Conversation

silbinarywolf
Copy link
Contributor

This builds on:
#27445

"...And then send change to fix windows internal/poll.FD.WriteTo - together with making TestUDPZeroBytePayload run again."

Fixes #26668

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Sep 2, 2018
@gopherbot
Copy link

Message from Gerrit User 5137:

Patch Set 1:

(3 comments)

Thank you for working on this Jake!

I have made some suggestions for the commit message
as well as code changes, please take a look.

I'll also tag some reviewers too.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 12446:

Uploaded patch set 3: New patch set was added with same tree, parent, and commit message as Patch Set 2.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 28917:

Patch Set 4: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 28917:

Patch Set 4:

(2 comments)

I've made the changes you've requested and altered the commit message for clarity.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 12446:

Uploaded patch set 5: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 12446:

Uploaded patch set 6: New patch set was added with same tree, parent, and commit message as Patch Set 5.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 12446:

Uploaded patch set 7: New patch set was added with same tree, parent, and commit message as Patch Set 6.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 5070:

Patch Set 7: Code-Review+1

Patch Set 4:

(2 comments)

I've made the changes you've requested and altered the commit message for clarity.

I do not see your commit message changes in Gerrit (in CL 132781).

I can see the change on Github in commit 3bf2b8b
But I don't think Github bot copies, commit message contents from Github into Gerrit. I think it copies PR (your PR is 27446) message. So, please, try and change your PR message instead.

Or, alternatively, update Gerrit directly. I use https://godoc.org/golang.org/x/review/git-codereview to communicate to Gerrit. I am certain, you could just use Git to access Gerrit, but I do not know what the commands are, and I do not want to confuse you more.

Your code change LGTM. Please change CL description, and someone will submit your change.

Thank you very much.

Alex


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@silbinarywolf silbinarywolf changed the title net: fix zero-byte payload bug in WriteTo() for windows net: ensure WriteTo on Windows sends even zero-byte payloads Sep 5, 2018
@gopherbot
Copy link

Message from Gerrit User 12446:

Uploaded patch set 8: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 12446:

Uploaded patch set 9: New patch set was added with same tree, parent, and commit message as Patch Set 8.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 28917:

Patch Set 9:

Patch Set 7: Code-Review+1

Patch Set 4:

(2 comments)

I've made the changes you've requested and altered the commit message for clarity.

I do not see your commit message changes in Gerrit (in CL 132781).

I can see the change on Github in commit 3bf2b8b
But I don't think Github bot copies, commit message contents from Github into Gerrit. I think it copies PR (your PR is 27446) message. So, please, try and change your PR message instead.

Or, alternatively, update Gerrit directly. I use https://godoc.org/golang.org/x/review/git-codereview to communicate to Gerrit. I am certain, you could just use Git to access Gerrit, but I do not know what the commands are, and I do not want to confuse you more.

Your code change LGTM. Please change CL description, and someone will submit your change.

Thank you very much.

Alex

Looks like editing the PR title worked.
Let me know if you need anything else!


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 5070:

Patch Set 9: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 5976:

Patch Set 9:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=a63962e1


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 5976:

Patch Set 9: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit User 5070:

Patch Set 9: Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/132781.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Sep 6, 2018
This builds on:
#27445

"...And then send change to fix windows internal/poll.FD.WriteTo - together with making TestUDPZeroBytePayload run again."
- alexbrainman - #26668 (comment)

Fixes #26668

Change-Id: Icd9ecb07458f13e580b3e7163a5946ccec342509
GitHub-Last-Rev: 3bf2b8b
GitHub-Pull-Request: #27446
Reviewed-on: https://go-review.googlesource.com/132781
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
@gopherbot
Copy link

This PR is being closed because golang.org/cl/132781 has been merged.

@gopherbot gopherbot closed this Sep 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

internal/poll: sending a zero-byte-payload UDP packet on Windows is a no-op
3 participants