[PATCH] D58801: [Support] Implement is_local_impl with AIX mntctl

[Zachary Turner via llvm-commits]

Given that the header is arguably defective, imo this unfortunate hack is
justifiable. Please leave a comment explaining it though
On Fri, Mar 15, 2019 at 3:54 PM Hubert Tong via Phabricator <
reviews at reviews.llvm.org> wrote:

> hubert.reinterpretcast added inline comments.
>
>
> ================
> Comment at: lib/Support/Unix/Path.inc:81
> +#include <sys/statfs.h>
> +typedef uint_t uint;
> +#include <sys/vmount.h>
> ----------------
> krytarowski wrote:
> > hubert.reinterpretcast wrote:
> > > krytarowski wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > krytarowski wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > apaprocki wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > apaprocki wrote:
> > > > > > > > > > `sys/statfs.h` includes `sys/types.h`, which contains
> this `typedef`. Was there a reason why it was put here between the two
> headers?
> > > > > > > > > Yes, the header is defective. It needs a typedef that is
> only conditionally defined.
> > > > > > > > I guess I am not seeing that.. is it OS version dependent?
> > > > > > > >
> > > > > > > >
> > > > > > > > ```
> > > > > > > > $ cat test.c
> > > > > > > > #include <sys/statfs.h>
> > > > > > > > #include <sys/vmount.h>
> > > > > > > > $ xlc -E -c test.c | grep "typedef.*uint;"
> > > > > > > > typedef         uint_t          uint;
> > > > > > > > $
> > > > > > > > ```
> > > > > > > Try with `-D_XOPEN_SOURCE=700` or anything that suppresses
> `_ALL_SOURCE`:
> > > > > > > ```
> > > > > > > > xlclang -D_XOPEN_SOURCE=700 test.c -c
> > > > > > > In file included from test.c:2:
> > > > > > > /usr/include/sys/vmount.h:195:2: error: unknown type name
> 'uint'; did you mean 'int'?
> > > > > > >         uint    vmt_revision;   /* I revision level, currently
> 1        */
> > > > > > >         ^
> > > > > > > /usr/include/sys/vmount.h:196:2: error: unknown type name
> 'uint'; did you mean 'int'?
> > > > > > >         uint    vmt_length;     /* I total length of structure
> and data */
> > > > > > >         ^
> > > > > > > /usr/include/sys/vmount.h:203:2: error: unknown type name
> 'uint'; did you mean 'int'?
> > > > > > >         uint    vmt_time;       /* O time of mount
>           */
> > > > > > >         ^
> > > > > > > /usr/include/sys/vmount.h:204:2: error: unknown type name
> 'uint'; did you mean 'int'?
> > > > > > >         uint    vmt_timepad;    /* O (in future, time is 2
> longs)       */
> > > > > > >         ^
> > > > > > > 4 errors generated.
> > > > > > > Error while processing test.c.
> > > > > > > Return:  0x01:1
> > > > > > > ```
> > > > > > This typedef looks suspicious, there shall be a way to
> workaround it.
> > > > > What is your proposed solution to otherwise provide a type name
> required by the system header that might not be defined? Note that this
> code is guarded under `defined(_AIX)`, and it has been demonstrated that
> this typedef might be defined anyway by the system headers on that platform
> to be an alias for `uint_t` (which is, in turn, an alias for `unsigned
> int`).
> > > > I don't have access right now to AIX to check it myself, but on UNIX
> it's typically needed to include `<sys/types.h>` and/or specify `-D` flags
> like `-D_POSIX_SOURCE`.
> > > Yes, and `<sys/types.h>` needs `-D_ALL_SOURCE` before `uint` is
> provided, and `_ALL_SOURCE` on AIX produces a lot of stray macros. I'd like
> to avoid needing `_ALL_SOURCE` for the build if at all possible.
> > How about restricting `_ALL_SOURCE` to this file only?
> >
> > ```
> > #define _ALL_SOURCE // Needed for uint typedef
> > #include <sys/statfs.h>
> > ```
> That would not work if `sys/types.h` is included by an earlier header, and
> defining `_ALL_SOURCE` earlier in `Path.cpp` would expose the LLVM headers
> included by that file to the stray macros that come with `_ALL_SOURCE`.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D58801/new/
>
> https://reviews.llvm.org/D58801
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190315/383dfc70/attachment.html>