On Wed, Feb 02, 2022 at 05:45:47PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richard...@intel.com] > > Sent: Wednesday, 2 February 2022 17.01 > > > > On Wed, Feb 02, 2022 at 07:54:58AM -0800, Stephen Hemminger wrote: > > > On Wed, 2 Feb 2022 09:47:32 +0000 > > > Sean Morrissey <sean.morris...@intel.com> wrote: > > > > > > > These header includes have been flagged by the iwyu_tool > > > > and removed. > > > > > > > > Signed-off-by: Sean Morrissey <sean.morris...@intel.com> > > > > --- > > [...] > > > > > > > > diff --git a/lib/pdump/rte_pdump.h b/lib/pdump/rte_pdump.h > > > > index 6efa0274f2..41c4b7800b 100644 > > > > --- a/lib/pdump/rte_pdump.h > > > > +++ b/lib/pdump/rte_pdump.h > > > > @@ -13,8 +13,6 @@ > > > > */ > > > > > > > > #include <stdint.h> > > > > -#include <rte_mempool.h> > > > > -#include <rte_ring.h> > > > > #include <rte_bpf.h> > > > > > > > > #ifdef __cplusplus > > > > > > This header does use rte_mempool and rte_ring in rte_pdump_enable(). > > > Not sure why IWYU thinks they should be removed. > > > > Because they are only used as pointer types, not as structures > > themselves. > > Normally in cases like this, I would put in just "struct rte_mempool;" > > at > > the top of the file rather than including a whole header just for one > > structure. > > I don't think we should introduce such a hack! > If a module uses something from a library, it makes sense to include the > header file for the library. > > Putting in "struct rte_mempool;" is essentially copy-pasting from the > library, although only a structure. What happens if the type changes or > disappears, or depends on some #ifdef? It could have one type in some cases > and another type in other cases - e.g. the atomic counters in the mbuf once > had different types, depending on compile time flags. The copy-pasted code > would not get fixed if the type evolved over time.
By "struct rte_mempool;" I mean literally just that. All it does is indicate that there is a structure defined somewhere else that will be used via pointer in the file later on. There is no copy-pasting involved and the reference does not need to change as the structure changes. >From what I read, having this forward declaration is not necessary for C, but for C++ if you use the struct pointer in a function definition later on, you may get an error. Therefore, if you are using a struct only as a pointer parameter, the best option is to forward declare it (to keep C++ happy), and not include a whole header file unnecessarily. /Bruce
