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

Minor type nits in string arguments example #72

Open
noncombatant opened this issue Oct 25, 2018 · 2 comments
Open

Minor type nits in string arguments example #72

noncombatant opened this issue Oct 25, 2018 · 2 comments

Comments

@noncombatant
Copy link

Re: http://jakegoulding.com/rust-ffi-omnibus/string_arguments/

Rust count returns usize; rather than (potentially down-)casting to uint32_t, does it make sense to change the example to return size_t? And then you don't need a cast.

In the C code, the printf format should be %u, or %zu if you use size_t. (Also you'll want to use %u in /slice_arguments/, and in /tuples. Or use int32_t, for which %d is appropriate.)

@Enet4
Copy link
Contributor

Enet4 commented Nov 6, 2018

The second part of this has been addressed in #76. About returning a count usize, it sounds reasonable, since it's heavily related to the size of some object in memory. But I wouldn't find either one particularly incorrect either, as long as the output is sure not to overflow a uint32_t. Maybe this is just a decision to be made on a case-by-case basis, depending on whether you wish to abstract away from the (potentially awkward) definition of size_t. There would be cases where this is unambiguous, of course: if the library was providing the number of views on a YouTube video, we'd totally go for a uint64_t instead.

@Stargateur
Copy link
Contributor

In the C code, the printf format should be %u, or %zu if you use size_t.

%u is not correct if used for size_t, you must use the z specifier.

Or use int32_t, for which %d is appropriate.)

No, d is not appropriate for int32_t, one correct could be PRId32

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

No branches or pull requests

4 participants