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

Made the dependency on stdlib.h and assert.h optional #10

Merged
merged 2 commits into from
Jun 25, 2020

Conversation

namandixit
Copy link
Contributor

@namandixit namandixit commented Jun 6, 2020

This PR will make it possible for users to provide their own malloc(), free() and assert() wrappers. This means that using this library on Windows (if one plans on shipping without MSVCRT Redist) or on embedded platforms (without a libc) will be easier.

The dependency on mman.h, windows.h and unistd.h are not touched, since I can't see why one might not want to use them on the relevant platforms. stdint.h is also left in since it only provides type definitions and not any functions.

The dependencies are still mandatory in the following files:

  1. sjlj.c: setjmp/longjmp are libc features anyway
  2. ucontext.c: No reason not to link to libc on POSIX systems
  3. ppc.c: Still uses string.h for memcpy. Since memcpy is usually a compiler intrinsic with some spec-mandated magical behaviour, not sure if it's technically safe to override it.

Some platforms still depend on Libc (if they are exclusively POSIX/Unix/Linux),
or need some other functionality (e.g., memcpy)
@Screwtapello
Copy link
Contributor

This makes sense to me, but I think the repeated stanza about #if !defined(LIBCO_...) should go into settings.h, where people who want to customise libco might look for settings.

Also, __VA_ARGS__ is apparently C99, while the rest of the codebase is (according to the README) C89. However, I don't know if that's a problem in practice. If you're working on embedded systems, you probably know more about the quirks of niche toolchains than I do.

@namandixit
Copy link
Contributor Author

Oh yeah, it should go into settings.h. And you are right about __VA_ARGS__ too (we don't really need it, I guess I should stop being cheeky :)

@namandixit
Copy link
Contributor Author

Should be good to go now

@Screwtapello
Copy link
Contributor

That's a thumbs-up from me, but I'll wait a day or so to see if any other maintainers have any objections before hitting the button.

@namandixit
Copy link
Contributor Author

Is there anything blocking this PR?

@Kawa-oneechan
Copy link
Contributor

Nah, not really.

@Kawa-oneechan Kawa-oneechan merged commit 1931c7d into higan-emu:master Jun 25, 2020
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.

4 participants