Skip to content

Conversation

@nomeata
Copy link
Contributor

@nomeata nomeata commented Jul 15, 2019

I am not a great expert on these matters, but at least here (compiling
for WebAssembly (wasm32-unknown-unknown-wasm), I need this, otherwise
size_t is not available.

@nijtmans
Copy link
Collaborator

Well, I don't think that's right: size_t is used in tommath.h, so - at least - whatever header file provides it should be included at the top of tommath.h somewhere.

Second, the header providing size_t is <stddef.h>, so including that in tommath.h should be the 'correct' thing to do. IMHO.

@nomeata nomeata force-pushed the stdlib branch 2 times, most recently from b283ae1 to ce25ee6 Compare July 15, 2019 13:54
@nomeata
Copy link
Contributor Author

nomeata commented Jul 15, 2019

Ah, thanks for educating me about stddef.h. That works indeed; updated this PR

@nomeata nomeata changed the title Import stdlib in tommath_private.h Import stddef in tommath_private.h Jul 15, 2019
@nijtmans nijtmans self-requested a review July 15, 2019 14:18
Copy link
Collaborator

@nijtmans nijtmans left a comment

Choose a reason for hiding this comment

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

Yes, this is the 'right' thing to do, IMHO. B.T.W.: I never noted this in libtommath, because I simply always add <stddef.h> first in my header files, but every header file should be self-contained, so it includes header-files which contain definitions it needs.

@nijtmans nijtmans assigned sjaeckel and unassigned sjaeckel Jul 15, 2019
@nijtmans nijtmans requested a review from sjaeckel July 15, 2019 14:22
@sjaeckel sjaeckel changed the title Import stddef in tommath_private.h Import stddef in tommath.h Jul 17, 2019
At least here (compiling for WebAssembly
(`wasm32-unknown-unknown-wasm`), I need this, otherwise `size_t` is not
available.
@sjaeckel sjaeckel merged commit 18c919f into libtom:develop Jul 17, 2019
@nomeata nomeata deleted the stdlib branch July 17, 2019 14:09
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