RFR 9: 8074818: Resolve disabled warnings for libjava

Martin Buchholz martinrb at google.com
Wed May 27 16:54:30 UTC 2015


Evil header file io_util.h effectively requires its callers to include
fcntl.h first (on platforms where fcntl.h defines O_SYNC and D_SYNC) and
that is a BIG NONO.  I would try hard to fix io_util.h now (instead of
adding workarounds to callers) and further, if any of its callers were
including fcntl.h merely for io_util.h's benefit, remove those include
statements.  IWYU!

Admittedly, google has far more IWYU religion than most.
https://code.google.com/p/include-what-you-use/

On Wed, May 27, 2015 at 7:24 AM, Roger Riggs <Roger.Riggs at oracle.com> wrote:

>  Hi Martin,
>
> Untangling the past a bit.  Perhaps this code could be cleaner but
> priority-wise,
> I've got some other things to do first.
>
> The Windows fcntl.h does not define O_SYNC/O_DSYNC so its relative include
> order is not significant.
> The explicit define of O_SYNC and O_DSYNC make the API to the Unix and
> Windows
> file support APIs consistent.
>
> On Windows file access is done using CreateFileW to get the native Windows
> semantics.
> The O_SYNC/DSYNC flags are mapped to the corresponding flags/attributes to
> CreateFileW.
>
> Roger
>
>
>
> On 5/27/2015 2:59 AM, Martin Buchholz wrote:
>
>
>
> On Tue, May 26, 2015 at 7:52 PM, Roger Riggs <Roger.Riggs at oracle.com>
> wrote:
>
>>  Hi,
>>
>> Sadly, but not entirely unexpectedly there is an anomaly in the include
>> files:
>> It seems that Windows does not define O_SYNC and O_DSYNC.
>> To make up for the absence
>> jdk/src/java.base/share/native/libjava/io_util.h
>> conditionally defines them.  There is no problem if the system include
>> files appear
>> first, but  in the other order, fcntl.h tries to re-define it.
>> In the recommended order, there is no issue.
>>
>
>  We should work hard to remove order dependencies in include files.
>
>  I see that io_util.h includes <fcntl.h>, but only on BSD.  Why not
> include it wherever it is available, (which may be all supported
> platforms!) before trying to define O_SYNC and D_SYNC?
>
>
>



More information about the core-libs-dev mailing list