This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Sharing of bits/fcntl.h on Linux (SPARC, x86)
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Andreas Jaeger <aj at suse dot com>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Wed, 17 Oct 2012 15:17:57 -0700 (PDT)
- Subject: Re: Sharing of bits/fcntl.h on Linux (SPARC, x86)
- References: <507F0BCA.8030902@suse.com>
> 2012-10-17 Andreas Jaeger <aj@suse.de>
>
> * sysdeps/unix/sysv/linux/bits/fcntl-linux.h: New file, contains
> generic values for Linux.
>
> * sysdeps/unix/sysv/linux/x86/bits/fcntl.h: Remove all definitions
> and declarations that are provided by <bits/fcntl-linux.h> and
> include <bits/fcntl-linux.h>.
> * sysdeps/unix/sysv/linux/sparc/bits/fcntl.h: Likewise.
If these are separate paragraphs, then their order is reversed (it's
reverse chronological order, and bits/fcntl-linux.h has to be there before
bits/fcntl.h files can change). But if you'll make it a single commit,
then make it a single paragraph.
> +#ifndef O_CREAT
> +# define O_CREAT 0100 /* Not fcntl. */
> +#endif
There should be a comment at the top saying that all the #ifndef cases are
to permit a machine-specific bits/fcntl.h to provide alternate definitions.
If there are any macros that are completely omitted from bits/fcntl-linux.h
so that each bits/fcntl.h is obliged to define them, then there should be a
comment listing all those so that the contract for using bits/fcntl-linux.h
is clearly documented.
> +/* For now Linux has synchronisity options for data and read operations.
"synchronicity"
> + We define the symbols here but let them do the same as O_SYNC since
> + this is a superset. */
> +#if defined __USE_POSIX199309 || defined __USE_UNIX98
> +# define O_DSYNC __O_DSYNC /* Synchronize data. */
> +/* For now, Linux has no separate synchronisitiy options for read
> + operations. We define O_RSYNC therefore as the same as O_SYNC
> + since this is a superset. */
Merge this with the comment above.
> +/* For old implementation of bsd flock(). */
Put BSD in caps. Don't use () to refer to a function name.
> +/* Operations for bsd flock(), also used by the kernel implementation. */
Likewise, and end with two spaces, not a tab.
> +++ b/sysdeps/unix/sysv/linux/sparc/bits/fcntl.h
> @@ -1,6 +1,5 @@
> /* O_*, F_*, FD_* bit values for Linux/SPARC.
> - Copyright (C) 1995-1998, 2000, 2003, 2004, 2006, 2007, 2009, 2010, 2011
> - Free Software Foundation, Inc.
> + Copyright (C) 1995-2012 Free Software Foundation, Inc.
Only one space between years and "Free".
> +#include <bits/fcntl-linux.h>
Precede this with a comment saying what it does.
> --- a/sysdeps/unix/sysv/linux/x86/bits/fcntl.h
> +++ b/sysdeps/unix/sysv/linux/x86/bits/fcntl.h
[...]
> @@ -25,57 +25,12 @@
> # include <bits/uio.h>
> #endif
Both the #include's belong in the common file.
The machine-specific file should have nothing but macro and type definitions.
I didn't actually check that all the bit values match.
Thanks,
Roland