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

Change FB_NELEM return type to unsigned and resolve all FB_NELEM related Wsign-compare warnings #8338

Merged
merged 3 commits into from
Dec 25, 2024

Conversation

Noremos
Copy link
Contributor

@Noremos Noremos commented Dec 4, 2024

FB_NELEM returns the size of the input array, which is always unsigned, but the function returns an int. It is a bit confusing, so I decided to change the type. It fixed some Wsign-compare warnings but added a bunch of new ones. Therefore, I included resolving them in the PR.

Copy link
Member

@dyemanov dyemanov left a comment

Choose a reason for hiding this comment

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

I've added a few minor comments, but I'm not going to insist if you consider it unnecessary.

src/jrd/filters.cpp Outdated Show resolved Hide resolved
src/jrd/par.cpp Outdated Show resolved Hide resolved
src/yvalve/gds.cpp Outdated Show resolved Hide resolved
src/yvalve/gds.cpp Outdated Show resolved Hide resolved
@Noremos
Copy link
Contributor Author

Noremos commented Dec 4, 2024

I've missed one warning in the sort.cpp file and fixed it in the last commit

@aafemt
Copy link
Contributor

aafemt commented Dec 4, 2024

A lot of conflict will be caused by this PR to others. Perhaps they should be merged first.

@Noremos
Copy link
Contributor Author

Noremos commented Dec 23, 2024

A lot of conflict will be caused by this PR to others. Perhaps they should be merged first.

Hello. I think enough time has passed to merge it

@aafemt
Copy link
Contributor

aafemt commented Dec 24, 2024

Still over 60 pull requests in the queue before yours.

@dyemanov
Copy link
Member

dyemanov commented Dec 24, 2024

Still over 60 pull requests in the queue before yours.

True, but they're reviewed/merged out of order. If some of them are affected (I highly doubt there will be many), they'll need to be updated before merging.

@dyemanov dyemanov merged commit c1707c4 into FirebirdSQL:master Dec 25, 2024
24 checks passed
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.

5 participants