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

Fixed implicit casts from size_t to int. #2

Closed

Conversation

neonichu
Copy link

Mostly by actually using size_t, except for the cases where negative values were considered valid, there I have stuck with long. The latter is of course all potential bugs, but at least they are more explicit now.

This makes it nicer to use cmark on iOS where -Wshorten-64-to-32 is a default warning.

@jgm
Copy link
Member

jgm commented Jan 30, 2015

@nwellnhof, I'd be interested in your feedback on the proposed change.

@neonichu, this seems incomplete - note the warnings on the travis build:
https://travis-ci.org/jgm/cmark/builds/48927694

@neonichu
Copy link
Author

@jgm oh, it indeed is. It appears I didn't have -Wsign-compare set, will amend.

Mostly by actually using `size_t`, except for the cases where negative
values were considered valid, there I have stuck with `long`. The latter
is of course all potential bugs, but at least they are more explicit
now.
@neonichu neonichu force-pushed the fix-implicit-32-64-casts branch from 421e1e4 to a945be4 Compare January 30, 2015 19:11
@nwellnhof
Copy link
Contributor

I would prefer a patch that doesn't change the types of struct members or function parameters unless there's a really good reason. Using size_t for things like header levels just to fix some warnings isn't the right approach.

Instead, I'd propose to add explicit casts in the places where the warnings appear, and maybe add a TODO comment if the cast looks unsafe. Changing the types of local variables is OK.

@jgm
Copy link
Member

jgm commented Feb 1, 2015

+++ Nick Wellnhofer [Jan 30 15 13:43 ]:

I would prefer a patch that doesn't change the types of struct members or function parameters unless there's a really good reason. Using size_t for things like header levels just to fix some warnings isn't the right approach.

Instead, I'd propose to add explicit casts in the places where the warnings appear, and maybe add a TODO comment if the cast looks unsafe. Changing the types of local variables is OK.

This sounds like the right approach to me, too.

@nwellnhof
Copy link
Contributor

This should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants