-
-
Notifications
You must be signed in to change notification settings - Fork 443
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
Clean up some code duplication in the header files #123
Conversation
PR htop-dev#70 got rid of the infrastructure for generating header files, but it left behind some code duplication. Some of cases are things that belong in the header file and don't need to be repeated in the C file. Other cases are things that belong in the C file and don't need to be in the header file. In this commit I tried to fix all of these that I could find. When given a choice I preferred keeping things out of the header file, unless they were being used by someone else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also have a look if we could utilize IWYU …
Using an static analysis tool to further clean up the headers sounds like an interesting idea but maybe it would be more appropriate for a different pull request? For this one I just used a script to identify lines of code that were repeated in both the .c and .h file. |
As suggested by cppclean.
I ran the code through cppclean. There were only a handful of includes that were detected as truly unused so I went ahead and removed them. Cppclean also pointed out several situations where we are only including a header file to be able to reference a pointer type from the header file. In theory those includes could be replaced by a forward declaration but I'm not sure if it would be worth the hassle. At least not for this PR, I guess.
|
@hugomg Only suggested IWYU for future work, as building with it can sometimes be a bit of a hassle. I'll open another issue for consideration and further investigation. Regarding pointer to structs: I usually don't really like having bare forward declarations floating around, unless the headers that would have to be included otherwise would be some massive overhead. As things are now you'd probably won't even notice the additional disk IO, even when trying to compile with your storage on the far side of a slow network … |
Oh... of course. My impression with respect to the header files is that the compilation time for each file is short enough that shaving off some includes doesn't make a big difference. I suspect that the more impactful change would be things that reduce the number of C files that need to be recompiled when a header file changes. For example, refactoring a class to remove the definition of the struct from the header file, making the fields private. I'm not sure if it would be worth the trouble but it would be something to keep in mind. |
IMHO compilation is fast enough as is. No need to put much work in there. |
PR #70 got rid of the infrastructure for generating header files, but it left behind some code duplication.
Some of cases are things that belong in the header file and don't need to be repeated in the C file. Other cases are things that belong in the C file and don't need to be in the header file.
In this commit I tried to fix all of these that I could find. When given a choice I preferred keeping things out of the header file, unless they were being used by someone else.