[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #25752 [Core Tor/Tor]: Detangle our included headers and reduce reliance on or.h
#25752: Detangle our included headers and reduce reliance on or.h
-------------------------------------------------+-------------------------
Reporter: isis | Owner: (none)
Type: enhancement | Status: new
Priority: Medium | Milestone: Tor:
| 0.3.5.x-final
Component: Core Tor/Tor | Version:
Severity: Normal | Resolution:
Keywords: tor-modularity, tor-compatibility, | Actual Points:
static-analysis |
Parent ID: | Points: 5
Reviewer: | Sponsor:
| Sponsor8-can
-------------------------------------------------+-------------------------
Description changed by isis:
Old description:
> Over the years, our code has grown to have a lot of `#include`s which are
> no longer necessary, and others which are over the scope of what actually
> needs to be included. A prime example of this is that nearly everything
> in `/src/or` includes `or.h` which is a huge, insanely long header file
> with nearly every type we've ever made. It'd be nice if we could decouple
> things a bit more.
>
> I made brief foray into playing with automated tools for doing this last
> week. First I tried https://github.com/myint/cppclean, but it wasn't able
> to process any of our code without displaying a python traceback, so I
> moved on to https://include-what-you-use.org/. iwyu worked a bit better,
> but it really wanted to change system headers and remove the defined
> safeguards (e.g. `#ifdef HAVE_SYS_SOCKET_H #include <sys/socket.h>
> #endif`), which feels super scary and I'm pretty sure will result in a
> lot of weird breakage, particularly on systems of lower tier support
> status and systems developers tend to not use (e.g. win32). There's a
> bunch of pragmas (https://github.com/include-what-you-use/include-what-
> you-use/blob/master/docs/IWYUPragmas.md) we could probably use to tell it
> to leave alone the system headers (e.g. `find src -iname "*.[ch]" -exec
> sed -i -e 's/#include <*>/#include <*> \/\/ IYWU pragma: keep/' {} \;` or
> something).
>
> It also wanted to do things like removing an `#include or.h` and then
> including from that only the system headers that were used, so maybe it's
> also a good idea to split up `or.h`? Like we could put the system headers
> and compatibility stuff at the top into a `prelude.h` or something that
> every module includes, and then more specific stuff in other header
> files. It might be nice to map out or graph which sections of code tend
> to need to use the same headers, in order to facilitate the organisation
> of splitting up `or.h`.
>
> I have a `feature/iwyu-test`
> [https://github.com/isislovecruft/tor/tree/feature/iwyu-test branch]
> where I committed the changes that iwyu made, plus some manual fixups
> that I made as I was perusing the changes (sorry, I should have kept
> those separate probably). I had to basically discard all changes to the
> ref10 ed25519 implementation, because it didn't understand why there were
> `#include`s mid-C-function. Also it wanted to `#include
> <bits/socket_type.h>` in several places, which had to be replaced with
> `#include <sys/socket.h>` instead, no idea why it wanted the private
> header in the first place (also if it did, why it didn't also `#define
> _SYS_SOCKET_H`). The output of `configure --disable-asciidoc && make -k
> CC=/home/isis/code/sources/iwyu/build/include-what-you-use
> 2>/tmp/iwyu.out` is attached. Also I compiled iwyu against LLVM/clang
> 3.5.
New description:
Over the years, our code has grown to have a lot of `#include`s which are
no longer necessary, and others which are over the scope of what actually
needs to be included. A prime example of this is that nearly everything in
`/src/or` includes `or.h` which is a huge, insanely long header file with
nearly every type we've ever made. It'd be nice if we could decouple
things a bit more.
I made brief foray into playing with automated tools for doing this last
week. First I tried https://github.com/myint/cppclean, but it wasn't able
to process any of our code without displaying a python traceback, so I
moved on to https://include-what-you-use.org/. iwyu worked a bit better,
but it really wanted to change system headers and remove the defined
safeguards (e.g. `#ifdef HAVE_SYS_SOCKET_H #include <sys/socket.h>
#endif`), which feels super scary and I'm pretty sure will result in a lot
of weird breakage, particularly on systems of lower tier support status
and systems developers tend to not use (e.g. win32). There's a bunch of
pragmas (https://github.com/include-what-you-use/include-what-you-
use/blob/master/docs/IWYUPragmas.md) we could probably use to tell it to
leave alone the system headers (e.g. `find src -iname "*.[ch]" -exec sed
-i -e 's/#include <*>/#include <*> \/\/ IYWU pragma: keep/' {} \;` or
something).
It also wanted to do things like removing an `#include or.h` and then
including from that only the system headers that were used, so maybe it's
also a good idea to split up `or.h`? Like we could put the system headers
and compatibility stuff at the top into a `prelude.h` or something that
every module includes, and then more specific stuff in other header files.
It might be nice to map out or graph which sections of code tend to need
to use the same headers, in order to facilitate the organisation of
splitting up `or.h`.
I have a `feature/iwyu-test`
[https://github.com/isislovecruft/tor/tree/feature/iwyu-test branch] where
I committed the changes that iwyu made, plus some manual fixups that I
made as I was perusing the changes (sorry, I should have kept those
separate probably). I had to basically discard all changes to the ref10
ed25519 implementation, because it didn't understand why there were
`#include`s mid-C-function. Also it wanted to `#include
<bits/socket_type.h>` in several places, which had to be replaced with
`#include <sys/socket.h>` instead, no idea why it wanted the private
header in the first place (also if it did, why it didn't also `#define
_SYS_SOCKET_H`). The output of `configure --disable-asciidoc && make -k
CC=/home/isis/code/sources/iwyu/build/include-what-you-use
2>/tmp/iwyu.out` is ~~attached~~
[https://people.torproject.org/~isis/docs/2018-04-09-bug25752-iwyu-
output.txt here]. Also I compiled iwyu against LLVM/clang 3.5.
--
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25752#comment:2>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs