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

build: fix -Wconversion/-Wsign-conversion warnings #12557

Closed
wants to merge 3 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Dec 19, 2023

Fix remaining warnings in examples and tests which are not suppressed
by the pragma in lib/curl_setup.h.

Silence a toolchain issue causing warnings in FD_SET() calls with
older Cygwin/MSYS2 builds. Likely fixed on 2020-08-03 by:
https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=5717262b8ecfed0f7fab63e2c09c78991e36f9dd

Follow-up to 2dbe75b #12492

Closes #12557


Red CI after merging to master: https://ci.appveyor.com/project/curlorg/curl/builds/48791215

Edit: The last CI run for the PR skipped the AppVeyor tests, which were not clean yet apparently.

@vszakats vszakats marked this pull request as draft December 19, 2023 19:04
@github-actions github-actions bot added Windows Windows-specific CI Continuous Integration tests labels Dec 19, 2023
@vszakats vszakats changed the title retest master after possible fallouts tests: fix -Wconversion warning Dec 19, 2023
@vszakats vszakats marked this pull request as ready for review December 19, 2023 19:28
@github-actions github-actions bot added the MQTT label Dec 19, 2023
The hope is that this fixes the FD_SET() type misalignment, that
is present in MSYS but not in Cygwin builds:

```
../../../docs/examples/sendrecv.c:47:3: warning: conversion to 'long unsigned int' from 'curl_socket_t' {aka 'int'} may change the sign of the result [-Wsign-conversion]
47 |   FD_SET(sockfd, &errfd); /* always check for error */
   |   ^~~~~~
```
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/48794906/job/54616tw3d74xrf6o?fullLog=true#L3031

This has been fixed in Cygwin in mid-2020:
https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=5717262b8ecfed0f7fab63e2c09c78991e36f9dd
@vszakats vszakats changed the title tests: fix -Wconversion warning build: fix -Wconversion/-Wsign-conversion warnings Dec 20, 2023
@vszakats vszakats closed this in 95a882d Dec 20, 2023
@vszakats vszakats deleted the retest branch December 20, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration MQTT tests Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

None yet

1 participant