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

Add stdio macros #25

Closed
wants to merge 0 commits into from
Closed

Add stdio macros #25

wants to merge 0 commits into from

Conversation

mandyedi
Copy link
Contributor

@mandyedi mandyedi commented Mar 2, 2019

I added macros for stdio.h (#8).
If you don't mind I used lowercase macros for types like FILE and stderr, but functions remained uppercase.

@AgentOak
Copy link
Contributor

AgentOak commented Mar 2, 2019

This is a really overengineered solution and I don't see it ever being used since implementing all these functions is impractical.

While #24 may be an okay workaround for the time being, the proper solution is to remove file handling altogether instead of sticking to it and making it more convoluted. fgets is not a particularly well designed function (see the necessary hack #24 to make it work properly) and we'd be better off not using it at all.

Instead we should let the caller specify a callback to request the material file, which will load the file into memory and return a pointer and size to tinyobj. That way the caller could use platform-specific functions like memory mapping.
Instead of using the inconvenient fgets, we can just search for the next newline character to find the end of the line and don't need to allocate memory at all.
This would fix not only #20, but #4 and #8 as well.

@syoyo
Copy link
Owner

syoyo commented Mar 2, 2019

Instead we should let the caller specify a callback to request the material file, which will load the file > into memory and return a pointer and size to tinyobj. That way the caller could use platform-specific > functions like memory mapping.
Instead of using the inconvenient fgets, we can just search for the next newline character to find the > end of the line and don't need to allocate memory at all.
This would fix not only #20, but #4 and #8 as well.

Yes, current implementation & design is a bit nasty. It is not recommended to extending it by macro approach. We are better to redesign & reimplement tinyobj-c by considering:

This will give much simpler and cleaner sour code.

@mandyedi
Copy link
Contributor Author

I accidentally closed it, but actually I decided to resolve the issue. As you suggested above I'm going to implement a callback.

@syoyo
Copy link
Owner

syoyo commented May 27, 2020

As you suggested above I'm going to implement a callback.

Thanks! 🙏

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