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

Remove obsolete typedef from compatibility file #303

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DarkWiiPlayer
Copy link

MinGWs ws2tcpip.h already defines socklen_t, making this redundant.
What's more, in a recent version of MinGW this has changed to unsigned int, breaking compatibility. See issue #302

I tried looking through the git log to see if there was a reason for this being there, but all I could find was in f3305405 it was added without a comment, under the commit message

Compiles and runs on linux and windows, using DLLs!

That was in 2003 using who knows which version of MinGW 😖

Thus, I assume it is OK to just remove this typedef altogether, but it'd be nice if someone could actually confirm this (Ideally, clarify why this was added in the first place)

MinGWs `ws2tcpip.h` already defines `socklen_t`, making this redundant.
What's more, in a recent version of MinGW this has changed to `unsigned int`, breaking compatibility.
@ewestbrook
Copy link
Contributor

ewestbrook commented Mar 30, 2020

Is there a macro we can test, for use in an #ifdef, in order to continue supporting older environments that might still require this typedef?

@DarkWiiPlayer
Copy link
Author

I actually have no idea. I went through the code and didn't find anything and I grepped for version, 53, 5.3 and a few other combinations to see if there was some convenient typedef somewhere in the codebase but didn't find anything.

But again, I don't know if it's even necessary for the typedef to be there in the first place.

@diegonehab
Copy link
Contributor

Why have we never heard of this problem when compiling under Windows? Is this a mingw problem?

@DarkWiiPlayer
Copy link
Author

Well, for one because the MinGW version that changed it was released in the beginning of this month (Version 5.3, iirc). Other than that, no idea. if by "compiling under windows" you mean microsofts compiler, I have never used that, so I couldn't tell you either.

@limjiregister
Copy link

How to resolve this issue?

Copy link
Member

@alerque alerque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel right. I'm not on Windows to test and poke around, but I don't see any logic for why this shouldn't be speced out here. Either it needs to be guarded behind some sort of check for whether it is already defined or something else needs to happen. Perhaps the correct way to resolve it is just let it be defined twice, but make it an unsigned int internally as well so we don't have conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants