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

Unnecessary includes #2666

Closed
Akuli opened this issue Jul 20, 2023 · 2 comments · Fixed by #2823
Closed

Unnecessary includes #2666

Akuli opened this issue Jul 20, 2023 · 2 comments · Fixed by #2823
Milestone

Comments

@Akuli
Copy link

Akuli commented Jul 20, 2023

Many files in FAPI include <dirent.h> and <poll.h> (and probably other headers) unnecessarily:

akuli@akuli-desktop:/tmp/tpm2-tss/src/tss2-fapi$ git grep -E '#include <(dirent|poll)'
api/Fapi_List.c:#include <dirent.h>
fapi_int.h:#include <poll.h>
fapi_util.c:#include <dirent.h>
ifapi_helpers.c:#include <dirent.h>
ifapi_io.c:#include <poll.h>
ifapi_io.c:#include <dirent.h>
ifapi_keystore.c:#include <dirent.h>
ifapi_policy_store.c:#include <dirent.h>

If I comment out all except the ones in ifapi_io.c, FAPI still compiles, because other files call wrapper functions defined in ifapi_io.c.

For context, I now have two blank files named dirent.h and poll.h just to satisfy the unnecessary includes on a system that doesn't provide <dirent.h> and <poll.h>. I already have a custom implementation of ifapi_io.c, so the two dummy files would be unnecessary if includes were cleaned up.

This isn't really a big deal to me though. Feel free to close if you don't care :)

@joholl
Copy link
Collaborator

joholl commented Jul 20, 2023

Good point. Thanks for pointing that out :)

@JuergenReppSIT we might want to consider compiling with include-what-you-use in our ci to ensure include correctness. I might have a look at this once I'm available again.

@JuergenReppSIT
Copy link
Member

@JuergenReppSIT we might want to consider compiling with include-what-you-use in our ci to ensure include correctness. I might have a look at this once I'm available again.

Yes that's a good idea.

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

Successfully merging a pull request may close this issue.

4 participants