[PATCH] D58427: Fix checking for rpc/xdr.h

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 1 23:25:36 PDT 2019


MaskRay added a comment.

In D58427#1447978 <https://reviews.llvm.org/D58427#1447978>, @ngie wrote:

> In D58427#1408294 <https://reviews.llvm.org/D58427#1408294>, @MaskRay wrote:
>
> > > The API on FreeBSD clearly calls for rpc/types.h: https://www.freebsd.org/cgi/man.cgi?query=xdr&sektion=3&apropos=0&manpath=freebsd .
> > > 
> > > By convention, FreeBSD splits up the types from the API declarations to avoid header pollution. This is why `sys/socket.h` (for instance) needing `sys/types.h` is more necessary on FreeBSD than Linux.
> >
> > Thanks for the explanation (to prevent header pollution), but for this particular case, I'm still not sure why https://github.com/freebsd/freebsd/blob/master/sys/rpc/xdr.h cannot include `rpc/types.h`. The file directly references `TRUE` `bool_t`. If it didn't, it would be justified to not include `rpc/includes.h`...
>
>
> @MaskRay: again, this is being done for historical reasons (~20 years worth of history) in terms of design choices, because of the philosophy that one should not need to require sys/types.h (in reality sys/_types.h) in multiple include headers, when documenting their inclusion once is sufficient, and generally done.
>
> It avoids having to pollute many headers, like rpc/xdr.h, with includes to rpc/types.h (the total count of rpc headers now being 28)...
>
>   $ ls -1 /usr/include/rpc/ | grep -vc types.h
>   28
>
>
> ... this reduces complexity and removes the need for having a split between multiple header directories, like Linux does, e.g., `bits/`, `asm-*/`, etc.
>
> It's not just FreeBSD that does this; many of the other BSDs follow this philosophy.




> again, this is being done for historical reasons (~20 years worth of history) in terms of design choices

I don't get the explanation why on FreeBSD, `rpc/xdr.h` not including `rpc/types.h` is justified. `sys/types.h`, Linux `/usr/include/{bits/,asm-*/}` are irrelevant.

Modern systems inheriting Sun RPC no longer require `rpc/xdr.h` to include `rpc/types.h`. glibc is one example, illumos (derived from OpenSolaris) https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/rpc/xdr.h is another.

Working around this issue for older FreeBSD or FreeBSD <= 12 is fine, but I'd like to see at least an attempt to address this issue in the FreeBSD upstream.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58427/new/

https://reviews.llvm.org/D58427





More information about the llvm-commits mailing list