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

Fix pointer mismatchings and other warnings #9823

Closed
wants to merge 1 commit into from

Conversation

AtariDreams
Copy link
Contributor

Many of these castings are unneeded if we change the variables to work better with each other.

Note that none of the public facing functions had their signature changed.

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

I review a part of this PR and it made me question what you are doing so I paused.

lib/base64.c Outdated Show resolved Hide resolved
lib/connect.c Outdated Show resolved Hide resolved
lib/connect.c Show resolved Hide resolved
lib/connect.c Outdated Show resolved Hide resolved
lib/curl_fnmatch.c Show resolved Hide resolved
lib/curl_fnmatch.c Show resolved Hide resolved
lib/curl_fnmatch.c Show resolved Hide resolved
@bagder
Copy link
Member

bagder commented Oct 29, 2022

quite bad

@AtariDreams
Copy link
Contributor Author

Not anymore

tests/server/util.c Outdated Show resolved Hide resolved
@bagder
Copy link
Member

bagder commented Oct 30, 2022

This PR starts to smell like a million tiny changes you think should be done, but most of them are now not about "pointer mismatches and other warnings". It's a busload of various unrelated changes that should not be in the same PR. It makes this PR very hard to review and next to impossible to ultimately merge.

Many of these castings are unneeded if we change the variables to work better with each other.

Note that none of the public facing functions had their signature changed.
@AtariDreams AtariDreams marked this pull request as draft October 30, 2022 17:32
jay pushed a commit to jay/curl that referenced this pull request Oct 31, 2022
Many of these castings are unneeded if we change the variables to work
better with each other.

Ref: curl#9823

Closes #xxxx
jay pushed a commit to jay/curl that referenced this pull request Oct 31, 2022
Many of these castings are unneeded if we change the variables to work
better with each other.

Ref: curl#9823

Closes #xxxx
@jay
Copy link
Member

jay commented Oct 31, 2022

You must have very strict compiler settings for the warnings you are seeing. Many of these changes are not needed.

We allow counter variables to go negative or wrap around. For example this is acceptable:

int foo = 5;
while(foo--) {
}
return

We allow subscript values to be a signed integer type. For example this is acceptable:

int foo = 5;
bar[foo]

We allow char * function parameters.

I'm closing this PR in favor of #9835 which is what is left after my review.

@jay jay closed this Oct 31, 2022
jay pushed a commit to jay/curl that referenced this pull request Oct 31, 2022
Many of these castings are unneeded if we change the variables to work
better with each other.

Ref: curl#9823

Closes #xxxx
jay pushed a commit to jay/curl that referenced this pull request Oct 31, 2022
Many of these castings are unneeded if we change the variables to work
better with each other.

Ref: curl#9823

Closes #xxxx
jay pushed a commit that referenced this pull request Nov 8, 2022
Many of these castings are unneeded if we change the variables to work
better with each other.

Ref: #9823

Closes #9835
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.

None yet

3 participants