Re: [PATCH 00/29] Remove some unused includes
On Wed, May 11, 2022 at 09:55:01PM +0800, Peng Liang wrote:
Recently, I update the toolchain in my dev machine from LLVM13 to
LLVM14,
and I find that there are many unsed include headers in the libvirt. So
I try to remove them in this series.
So is clang actually reporting that the headers are unused, or is
there some other tool with LLVM14 that is reporting this. I'm
basically curious how you go about finding the redundant includes ?
I do wonder if we could automate reporting in CI, but then whether
a header is redundant or not, is likely to be platform specific.
ie freebsd might need a header but on Linux perhaps not, or vica
verca.
Besides, I also find that:
1. some header files are not self-contained, which means if you want to
include one header, you need to include more headers to meet the
requirements of the declarations in the header you want to include;
This is definitely a bug. We want all our headers to be self-contained
and should fix any such problems.
2. some includes in the .h file are not the dependences of the .h
file
(the declaration) but the dependences of the .c file (the
implementation), maybe it's better to move them to .c file.
Agreed, those would be better moved into the .c, as it could
(theoretically at least) speed up compilation to not huave so
many includes visible across the codebase.
But it will take more time to cleanup. So I only remove the unused
includs in this series. Is the community welcome to the removing and
the cleanup I mentioned above? If so, I'll move on and cleanup more.
Conceptually I think the cleanup is useful. Just have to be careful
not to break the code on platforms where different headers might be
needed to get the declaration for a given symbol. For example this
series breaks on Ubuntu 20.04 and Mingw64:
https://gitlab.com/berrange/libvirt/-/jobs/2443438983
https://gitlab.com/berrange/libvirt/-/jobs/2443439016
and indeed many other platforms with the same missing geteuid
declaration.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|