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

Jump to bottom

cmake: add missing inet_ntop check #9689

Closed
wants to merge 2 commits into from
Closed

cmake: add missing inet_ntop check #9689

wants to merge 2 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 10, 2022 •
edited

This adds the missing half of the check, next to the other half already present in lib/curl_config.h.cmake.

Force disable HAVE_INET_NTOP for old MSVC where it caused compiler warnings.

Reviewed-by: Daniel Stenberg

Closes #9689

@vszakats
cmake: add missing inet_ntop check
6d14d00 
This adds the missing half of the check, next to the other half
already present in `lib/curl_config.h.cmake`.
@vszakats vszakats added the cmake label Oct 10, 2022
@vszakats
Copy link
Member Author

vszakats commented Oct 10, 2022 •
edited

Results in two build warnings (promoted to errors in CI) with old MSVC envs:

@jay
Copy link
Member

jay commented Oct 10, 2022

It looks like in Windows earlier versions of the SDK (which would likely be used with those versions of Visual Studio) declared inet_ntop addr parameter without const qualfier. Here's from the Windows 7 SDK:

PCSTR
WSAAPI
inet_ntop(
    __in                                INT             Family,
    __in                                PVOID           pAddr,
    __out_ecount(StringBufSize)         PSTR            pStringBuf,
    __in                                size_t          StringBufSize
    );

curl/lib/hostip.c

Lines 152 to 169 in 93d0928

switch(ai->ai_family) {
case AF_INET: {
const struct sockaddr_in *sa4 = (const void *)ai->ai_addr;
const struct in_addr *ipaddr4 = &sa4->sin_addr;
(void)Curl_inet_ntop(ai->ai_family, (const void *)ipaddr4, buf, bufsize);
break;
}
#ifdef ENABLE_IPV6
case AF_INET6: {
const struct sockaddr_in6 *sa6 = (const void *)ai->ai_addr;
const struct in6_addr *ipaddr6 = &sa6->sin6_addr;
(void)Curl_inet_ntop(ai->ai_family, (const void *)ipaddr6, buf, bufsize);
break;
}
#endif
default:
break;
}

@vszakats
Copy link
Member Author

@jay: Indeed. If you have a patch for that, we could include it, otherwise I had force-disabled this detection for these MSVC versions (tests still pending).

@vszakats vszakats closed this in 3b48374 Oct 11, 2022
@vszakats vszakats deleted the cmake-ntop branch October 11, 2022 07:44
vszakats added a commit to curl/curl-for-win that referenced this pull request Oct 11, 2022
@vszakats
curl-cmake.sh: simplify after upstream curl fixes [ci skip]
8de1c92 
Upcoming curl release received fixes to correctly detect these build
flags, so drop manual settings for them:

- `HAVE_UNISTD_H` and with it: `HAVE_FTRUNCATE`
- `HAVE_STRTOK_R`
- `HAVE_STRCASECMP`
- `HAVE_INET_NTOP`

Ref: curl/curl#9687
Ref: curl/curl#9689
obonaventure pushed a commit to mptcp-apps/curl that referenced this pull request Oct 12, 2022
@vszakats @obonaventure
cmake: add missing inet_ntop check
edbb2a9 
This adds the missing half of the check, next to the other half
already present in `lib/curl_config.h.cmake`.

Force disable `HAVE_INET_NTOP` for old MSVC where it caused compiler
warnings.

Reviewed-by: Daniel Stenberg

Closes curl#9689
@1234567808 1234567808 mentioned this pull request Nov 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
cmake
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@vszakats @jay @bagder