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

eliminate inclusion loops in safe_mem_lib.h and safe_str_lib.h #14

Closed
jeremyhannon opened this issue Aug 31, 2017 · 6 comments
Closed

Comments

@jeremyhannon
Copy link
Contributor

I'm concerned that there are inclusion loops. Specifically, safe_mem_lib.h and safe_str_lib.h both include safe_lib.h which then includes both of them. I assume this is because safe_lib.h defines rsize_t and the abort/ignore handlers. I would suggest that rsize_t be moved into safe_types.h and maybe the handlers need to be put in their own .c/.h files. That way, safe_mem_lib.h and safe_str_lib.h can include safe_types.h and the safe_handlers.h but that's it. Then safe_lib.h will include safe_str_lib.h and safe_mem_lib.h.

Any issue with that restructuring to eliminate inclusion loops?

@rurban
Copy link
Owner

rurban commented Aug 31, 2017

sounds good. c++ is also a good check for finding such recursion errors.

@Juno-Explorer
Copy link
Contributor

👍🏻

rurban added a commit that referenced this issue Sep 1, 2017
hide the UNSAFE functions with --disable-unsafe.

Also resolve the header inclusion loops from #14
rurban added a commit that referenced this issue Sep 1, 2017
hide the UNSAFE functions with --disable-unsafe.

Also resolve the header inclusion loops from #14
rurban added a commit that referenced this issue Sep 1, 2017
hide the UNSAFE functions with --disable-unsafe.

Also resolve the header inclusion loops from #14
rurban added a commit that referenced this issue Sep 1, 2017
hide the UNSAFE functions with --disable-unsafe.

Also resolve the header inclusion loops from #14
rurban added a commit that referenced this issue Sep 1, 2017
hide the UNSAFE functions with --disable-unsafe.

Also resolve the header inclusion loops from #14
@rurban
Copy link
Owner

rurban commented Sep 7, 2017

Note that I had to change the includes a bit for windows support.
Currently I'm fine with safe_lib.h being included by the 2 others, and vice versa.
Just doxygen needs to get 2 more cpp defines to stop worrying.

@rurban
Copy link
Owner

rurban commented Sep 27, 2017

Done with 234f3b0

@rurban rurban closed this as completed Sep 27, 2017
@jeremyhannon
Copy link
Contributor Author

jeremyhannon commented Sep 27, 2017

234f3b0 doesn't get rid of the inclusion loops. safe_str_lib.h still includes safe_lib.h. I envision the fix is to remove all types from safe_lib.h so that safe_str_lib.h doesn't need to depend on it at all. safe_lib.h is just the umbrella "include all" header.

@rurban rurban reopened this Sep 27, 2017
rurban added a commit that referenced this issue Oct 2, 2017
you need to include the right header file now,
either safe_str_lib.h, safe_mem_lib.h or safe_lib.h for the rest.
Similar behaviour as before, though a bit complex.

Closes GH #14
rurban added a commit that referenced this issue Oct 3, 2017
you need to include the right header file now,
either safe_str_lib.h, safe_mem_lib.h or safe_lib.h for the rest.
Similar behaviour as before, though a bit complex.

Closes GH #14
@rurban
Copy link
Owner

rurban commented Oct 4, 2017

Fixed with fb4a6d5

@rurban rurban closed this as completed Oct 4, 2017
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

No branches or pull requests

3 participants