cvs commit: src/include stdio.h src/sys/sys unistd.h

[Sean Bryant]

Andrey Chernov wrote:
> On Sat, Apr 07, 2007 at 06:54:35PM +0200, Pawel Jakub Dawidek wrote:
>   
>> On Sat, Apr 07, 2007 at 08:40:22PM +0400, Andrey Chernov wrote:
>>     
>>> On Sat, Apr 07, 2007 at 04:02:31PM +0000, Pawel Jakub Dawidek wrote:
>>>       
>>>>   - Only define SEEK_DATA and SEEK_HOLE in sys/unistd.h when neither
>>>>     _POSIX_SOURCE nor _XOPEN_SOURCE is defined.
>>>>         
>>> 1) This new addition should be exluded for !define(_C99_SOURCE)
>>> !define(_ANSI_SOURCE) !define(_POSIX_C_SOURCE) too.
>>>
>>> 2) We design our *_VISIBLE framework right for the reason to not mention 
>>> all possible *_SOURCE each time like this, making includes unnecessary big 
>>> and hard to adapt to the future *_SOURCE tags, but mention one *_VISIBLE 
>>> tag instead, so please rewrite this thing using it.
>>>       
>> That's why I asked for help. _CDDL_VISIBLE is not good, because it is
>> not related to CDDL license. _ZFS_VISIBLE is not good, because it is not
>> ZFS-specific. _{SUN,SOLARIS,OPENSOLARIS}_VISIBLE is also not good,
>> because it is already in Linux too. Solaris simply defines
>> __EXTENSIONS__. Maybe we need something like this? I don't think we need
>> separate _*_VISIBLE defines for every new #define in unistd.h and other
>> headers, so something similar to __EXTENSIONS__ makes sense to me.
>>     
>
> Ever for simple few defines *_VISIBLE framework is better because it is 
> easily extensible to new possible *_SOURCE comes without needs to edit 
> _all_ includes where namespaces user after that (i.e. without adding new 
> !defined(_XXX_SOURCE) to all there).
>
> __EXTENSIONS__ scope is too large and don't specific to be meaningful.
> Why _{SUN,SOLARIS,OPENSOLARIS}_VISIBLE isn't good? If it is SOLARIS 
> permanent addition, it and possible other ones from there perfectly fits 
> IMHO.
>
> The patch below using __ZFS_VISIBLE as example, feel free to change to 
> better name.
>
> --- cdefs.h.bak	Sat Sep 23 18:59:06 2006
> +++ cdefs.h	Sat Apr  7 20:51:34 2007
> @@ -492,6 +492,7 @@
>  #undef _POSIX_C_SOURCE
>  #define	_POSIX_C_SOURCE		199506
>  #endif
> +#define	__ZFS_VISIBLE		0
>  #endif
>  
>  /*
> @@ -521,6 +522,7 @@
>  #define	__POSIX_VISIBLE		198808
>  #define	__ISO_C_VISIBLE		0
>  #endif /* _POSIX_C_SOURCE */
> +#define	__ZFS_VISIBLE		0
>  #else
>  /*-
>   * Deal with _ANSI_SOURCE:
> @@ -538,16 +540,19 @@
>  #define	__POSIX_VISIBLE		0
>  #define	__XSI_VISIBLE		0
>  #define	__BSD_VISIBLE		0
> +#define	__ZFS_VISIBLE		0
>  #define	__ISO_C_VISIBLE		1990
>  #elif defined(_C99_SOURCE)	/* Localism to specify strict C99 env. */
>  #define	__POSIX_VISIBLE		0
>  #define	__XSI_VISIBLE		0
>  #define	__BSD_VISIBLE		0
> +#define	__ZFS_VISIBLE		0
>  #define	__ISO_C_VISIBLE		1999
>  #else				/* Default environment: show everything. */
>  #define	__POSIX_VISIBLE		200112
>  #define	__XSI_VISIBLE		600
>  #define	__BSD_VISIBLE		1
> +#define	__ZFS_VISIBLE		1
>  #define	__ISO_C_VISIBLE		1999
>  #endif
>  #endif
> --- unistd.h.bak	Sat Apr  7 20:02:30 2007
> +++ unistd.h	Sat Apr  7 20:56:06 2007
> @@ -108,7 +108,7 @@
>  #define	SEEK_CUR	1	/* set file offset to current plus offset */
>  #define	SEEK_END	2	/* set file offset to EOF plus offset */
>  #endif
> -#if !defined(_POSIX_SOURCE) && !defined(_XOPEN_SOURCE)
> +#if __ZFS_VISIBLE
>  #define	SEEK_DATA	3	/* set file offset to next data past offset */
>  #define	SEEK_HOLE	4	/* set file offset to next hole past offset */
>  #endif
>
>
>
>   
I may be out of place here but Pawel did state he would be implementing 
SEEK_HOLE and SEEK_DATA on UFS. When that change makes it in, this 
becomes useless, correct? As Pawel stated __EXTENSIONS__ might make a 
bit more sense since these changes are not part of the CDDL additions 
nor are they part of ZFS. They are simply extensions the OS defines.

Those are just my two cents, feel free to ignore them as FreeBSD 
development isn't my area of expertise.