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

Make sure errno is set when socket operations fail #2405

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

dscho
Copy link
Member

@dscho dscho commented Nov 14, 2019

Our mingw_*() function family that works with sockets failed to set errno, as noticed in #2404.

@dscho
Copy link
Member Author

dscho commented Nov 14, 2019

@jeffhostetler I did not test this, like, at all. Could I ask you...? You seemed to have a concrete scenario where this was an issue, maybe you can cherry-pick and see whether this PR would fix it?

@jeffhostetler
Copy link

Sure. I'll look at it tomorrow as I'm testing what I'm working on now.

I wasn't sure if we wanted this in GFW or initially send it upstream.

@dscho
Copy link
Member Author

dscho commented Nov 14, 2019

Sure. I'll look at it tomorrow as I'm testing what I'm working on now.

Great, thanks!

I wasn't sure if we wanted this in GFW or initially send it upstream.

I'd like to test it in Git for Windows for a while first, then send it upstream.

@dscho
Copy link
Member Author

dscho commented Nov 20, 2019

@jeffhostetler ping?

@jeffhostetler
Copy link

yeah. i got distracted.... sorry.

@jeffhostetler
Copy link

Should we include bind() and listen() too ?

@jeffhostetler
Copy link

https://github.com/jeffhostetler/git/tree/mingw-setsockopt

I gave it a try on top of the stuff I'm currently working on and it seemed ok,
but I didn't go too deep on it. (I couldn't replicate the original problem that
I was having last month.)

I tweaked the WINSOCK_RETURN() macro a bit to make it easier to debug in GDB.

@dscho
Copy link
Member Author

dscho commented Nov 22, 2019

I tweaked the WINSOCK_RETURN() macro a bit to make it easier to debug in GDB.

How?

@jeffhostetler
Copy link

See my branch. I didn't modify your branch. I left in the fprintf's where I was testing it interactively.

@dscho
Copy link
Member Author

dscho commented Nov 22, 2019

See my branch.

Could you kindly point me to it? I lost track...

@dscho
Copy link
Member Author

dscho commented Nov 23, 2019

See my branch.

Could you kindly point me to it? I lost track...

Oy, I had to scroll back only 3 lines to see it. Sorry for the noise.

@dscho
Copy link
Member Author

dscho commented Nov 23, 2019

Should we include bind() and listen() too ?

And accept(), and actually all of the wrappers around winsock2 functions ;-)

@jeffhostetler could you have another look over the patch?

@jeffhostetler
Copy link

I just pushed another fixup to my branch and kicked off #2417 to let the CI tests run.

The winsock2 library provides functions that work on different data
types than file descriptors, therefore we wrap them.

But that is not the only difference: they also do not set `errno` but
expect the callers to enquire about errors via `WSAGetLastError()`.

Let's translate that into appropriate `errno` values whenever the socket
operations fail so that Git's code base does not have to change its
expectations.

This closes git-for-windows#2404

Helped-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho merged commit 8715ee2 into git-for-windows:master Nov 25, 2019
@dscho dscho added this to the Next release milestone Nov 25, 2019
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2019
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2019
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Nov 25, 2019
Make sure `errno` is set when socket operations fail
dscho added a commit that referenced this pull request Nov 26, 2019
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Nov 27, 2019
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
dscho added a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
dscho added a commit that referenced this pull request Jul 2, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 3, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 3, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 4, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 4, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 4, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 4, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 9, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 9, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
dscho added a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 10, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 11, 2024
Make sure `errno` is set when socket operations fail
dscho added a commit that referenced this pull request Jul 11, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 11, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 11, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 12, 2024
Make sure `errno` is set when socket operations fail
git-for-windows-ci pushed a commit that referenced this pull request Jul 12, 2024
Make sure `errno` is set when socket operations fail
dscho added a commit to dscho/git that referenced this pull request Jul 12, 2024
Make sure `errno` is set when socket operations fail
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants