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

Removes warnings about using deprecated inet function #10698

Merged
merged 4 commits into from
Aug 18, 2021

Conversation

supperpiccle
Copy link
Contributor

The warning in question is: "Warning C4996 'inet_ntoa': Use
inet_ntop() or InetNtop() instead or define
_WINSOCK_DEPRECATED_NO_WARNINGS"

Simply adding the _WINSOCK_DEPRECATED_NO_WARNINGS preprocessor silences
the warnings.

@Nekotekina
Copy link
Member

Well, this function is really problematic unless it stores its result in thread-local storage (it probably doesn't)

@supperpiccle
Copy link
Contributor Author

I would have imagined that if this caused issues, it would have already. If you would prefer, I'll just replace all uses of inet_ntoa to a safer function.

@Nekotekina
Copy link
Member

Replacing them should be safer. Alternatively, it could be implemented as a formatting function specialization for in_addr since it's required to be a big-endian 32-bit integer with somewhat obscure specification.

The warning in question is: "Warning	C4996	'inet_ntoa': Use
inet_ntop() or InetNtop() instead or define
_WINSOCK_DEPRECATED_NO_WARNINGS"

Simply adding the _WINSOCK_DEPRECATED_NO_WARNINGS preprocessor silences
the warnings.
@supperpiccle
Copy link
Contributor Author

Updated PR by removing the first commit and replacing all uses of inet_ntoa with inet_ntop.

@supperpiccle
Copy link
Contributor Author

supperpiccle commented Aug 15, 2021

Upon further reflection, it seems this code is called rarely enough that we could justify having a helper function that returns an std::string and we use the .c_str() member function for these logging messages. It will have to be a stack allocated variable though, but would remove the ugly inet_ntop calls and the assumption that we report ipv4 addresses.

Open to feedback here.

@RipleyTom
Copy link
Contributor

RipleyTom commented Aug 15, 2021

I mean that function already exists in the code, it's ip_to_string.
A specialization of formatter for in_addr seems also like a good idea to me(as the vast majority of the formatting is for those logging calls). Also no need to use .c_str(), the formatter can handle std::string fine.

@supperpiccle
Copy link
Contributor Author

I assumed that we were passing the format string + data to a printf family function. I did not know that rpcs3 had an implementation of format string generation.

While I agree having a format specifier that takes a in_addr seems cool, I have a feeling that this special specifier for this and leaving it undocumented will lead to us using it in this PR and forgetting about it. The solution might be to simply extend some of our dev docs to include these special format specifiers. Y'all may not agree with this sentiment in which case, I'll just make the change 😄 .

@Nekotekina Nekotekina merged commit c13a46b into RPCS3:master Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants