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

Should we use sized types instead of 'unsigned long' ? #15231

Open
richsalz opened this issue May 11, 2021 · 5 comments
Open

Should we use sized types instead of 'unsigned long' ? #15231

richsalz opened this issue May 11, 2021 · 5 comments
Labels
branch: master Merge to master branch triaged: OTC evaluated This issue/pr was triaged by OTC triaged: refactor The issue/pr requests/implements refactoring
Milestone

Comments

@richsalz
Copy link
Contributor

There are a lot of unsigned long parameters, return values, etc., in the header files. Should we move to sized types? Most of them could just be uint32_t See #15145. We are fixing that because we need more bits, but it might be worth
taking a look at the other API's. Most are probably okay with 32bit. This is similar to the long-wanted int/size_t cleanup, but would only affect 'unsigned long' uses and could be done almost mechanically.

@paulidale paulidale added this to the 3.0.0 beta1 milestone May 11, 2021
@paulidale paulidale added branch: master Merge to master branch hold: need otc decision The OTC needs to make a decision labels May 11, 2021
@paulidale
Copy link
Contributor

Flagging this for dicussion.

@h-vetinari
Copy link
Contributor

+1 for explicitly sized ints.

@levitte
Copy link
Member

levitte commented May 12, 2021

I would do that where it matters... it doesn't always.

@richsalz
Copy link
Contributor Author

I would do that where it matters... it doesn't always.

Sure. I'd expect to do one PR per header (or maybe related, like all ssl/tls in one)

@mattcaswell mattcaswell added the triaged: refactor The issue/pr requests/implements refactoring label May 13, 2021
@paulidale paulidale modified the milestones: 3.0.0 beta1, Post 3.0.0, 4.0.0 May 18, 2021
@paulidale paulidale added triaged: OTC evaluated This issue/pr was triaged by OTC and removed hold: need otc decision The OTC needs to make a decision labels May 18, 2021
@paulidale
Copy link
Contributor

We want to do this, we're not going to do it for 3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch triaged: OTC evaluated This issue/pr was triaged by OTC triaged: refactor The issue/pr requests/implements refactoring
Projects
None yet
Development

No branches or pull requests

5 participants