This is a follow up on my recent post about the include-what-you-use tool.
I discovered a way that you can use this tool to include a more appropriate
set of header files in the files that you develop than would otherwise be
the case.
The stated philosophy behind the tool is that you should directly include
all header files that are used by a file. If struct a is declared in a.h
and struct b is declared in b.h, your .c file that references both struct a
and struct b should directly include a.h and b.h. But this will lead to
including many more files than we have typically done in vpp. My personal
preference, although not sanctioned by the pros, is to allow indirect
header file inclusion. It turns out that there is a simple way to do this
using include-what-you-use, and it does not require rewriting the tool's
code.
include-what-you-use will suggest which header files should be added and
which header files should be removed from the file that you are analyzing.
Understand that the corresponding .h file to a .c file will be treated
specially. It will be analyzed along with the .c file.
For an example, I will use vnet/tcp/builtin_client.c. First I show files
that are suggested to be removed from builtin_client.h.
vnet/tcp/builtin_client.h should remove these lines:
- #include <svm/svm_fifo_segment.h> // lines 28-28
- #include <vnet/ip/ip.h> // lines 22-22
- #include <vnet/session/application_interface.h> // lines 30-30
- #include <vnet/session/session.h> // lines 29-29
- #include <vppinfra/error.h> // lines 26-26
- #include <vppinfra/hash.h> // lines 25-25
Running include-what-you-use involves running the clang C compiler, so if a
necessary header file is missing and a type cannot be resolved, you will
see regular compiler error messages.
After removing the header file includes above from builtin_client.h, and
re-running include-what-you-use, we find the error:
In file included from vnet/tcp/builtin_client.c:20:
./vnet/tcp/builtin_client.h:40:3: error: unknown type name 'svm_fifo_t'
svm_fifo_t *server_rx_fifo;
^
We manually search for the svm_fifo_t declaration and we see that rather
than including svm_fifo_segment.h in builtin_client.h, we should have
included svm_fifo.h.
Fixing that and re-running IWYU, we find
vnet/tcp/builtin_client.c:55:3: error: use of undeclared identifier
'session_fifo_event_t' session_fifo_event_t evt;
^
so therefore, session.h should have been included in builtin_client.c
rather than builtin_client.h.
We also find
vnet/tcp/builtin_client.c:258:8: error: use of undeclared identifier
'vnet_disconnect_args_t'
vnet_disconnect_args_t _a, *a = &_a;
^
so application_interface.h should have been included in builtin_client.c
rather than builtin_client.h.
Re-running IWYU tells us that no lines need to be removed from
builtin_client.h, however,
vnet/tcp/builtin_client.c should remove these lines:
- #include <vlibapi/api.h> // lines 24-24
- #include <vlibmemory/api.h> // lines 25-25
- #include <vlibsocket/api.h> // lines 26-26
- #include <vnet/plugin/plugin.h> // lines 19-19
- #include <vnet/vnet.h> // lines 18-18
- #include <vpp/app/version.h> // lines 27-27
Removing these includes, re-running IWYU indicates that no more includes
need to be removed from either builtin_client.h or builtin_client.c, and
the compilation is successful. We are done, and
we have
builtin_client.h includes:
#include <vnet/vnet.h>
#include <vnet/ethernet/ethernet.h>
#include <vlibmemory/unix_shared_memory_queue.h>
#include <svm/svm_fifo.h>
builtin_client.c includes:
#include <vnet/tcp/builtin_client.h>
#include <vnet/session/session.h>
#include <vnet/session/application_interface.h>
Now, if on the other hand, a developer prefers to include all the headers
directly, like many experts like to see, the result would be:
The full include-list for vnet/tcp/builtin_client.c:
#include <vnet/tcp/builtin_client.h>
#include <string.h> // for memset, NULL
#include <vnet/session/application_interface.h> // for
vnet_app_attach_args_t
#include <vnet/session/session.h> // for
stream_session_handle
#include "svm/svm_fifo.h" // for svm_fifo_t,
svm_fif...
#include "vlib/cli.h" // for vlib_cli_output
#include "vlib/global_funcs.h" // for
vlib_get_thread_main
#include "vlib/init.h" // for VLIB_INIT_FUNCTION
#include "vlib/node_funcs.h" // for
vlib_process_get_ev...
#include "vlib/threads.h" // for
vlib_get_thread_index
#include "vlibapi/api_common.h" // for api_main_t,
api_main
#include "vlibmemory/api_common.h" // for
vl_api_memclnt_crea...
#include "vlibmemory/unix_shared_memory_queue.h" // for
unix_shared_memory_...
#include "vnet/session/application.h" // for session_cb_vft_t
#include "vnet/session/stream_session.h" // for stream_session_t
#include "vppinfra/clib.h" // for PREDICT_FALSE,
ARRA...
#include "vppinfra/clib_error.h" // for clib_error_t
#include "vppinfra/elog.h" // for ELOG_DATA,
ELOG_TYP...
#include "vppinfra/error.h" // for clib_warning,
clib_...
#include "vppinfra/error_bootstrap.h" // for ASSERT
#include "vppinfra/format.h" // for unformat,
unformat_...
#include "vppinfra/memcpy_avx.h" // for clib_memcpy
#include "vppinfra/pool.h" // for pool_header_t,
pool...
#include "vppinfra/smp.h" // for
__sync_fetch_and_add_4
#include "vppinfra/vec.h" // for vec_validate,
vec_add1
#include "vppinfra/vec_bootstrap.h" // for vec_header_t,
vec_len
---
and
The full include-list for vnet/tcp/builtin_client.h:
#include <pthread.h> // for pthread_t
#include <svm/svm_fifo.h> // for svm_fifo_t
#include <vlibmemory/unix_shared_memory_queue.h> // for
unix_shared_memory_...
#include <vnet/ethernet/ethernet.h> // for ethernet_main_t
#include <vnet/vnet.h> // for vnet_main_t
#include <vppinfra/clib.h> // for u32, u64, u8, f64
#include "vlib/main.h" // for vlib_main_t
#include "vlib/node.h" // for
vlib_node_registrat...
#include "vppinfra/lock.h" // for clib_spinlock_t
---
Again, the beauty is that a developer can focus on just his or her files,
and come up with a short list or a long list of #includes.
I will discuss some additional fine points in a future post. In the
meantime, note that it is somewhat tricky to get the tool to give you
consistent usage of '"' versus '<' and '>'. Possible but a bit tricky or
annoying. Roughly it involves having the header files installed under
/usr/include, and not using -I. Anyway, here it was only an issue with the
long list of #includes.
Regards,
Burt
_______________________________________________
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev
