This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Sharing of bits/fcntl.h on Linux (SPARC, x86)


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]