Date: Sat, 07 Apr 2007 13:34:18 -0400 From: Sean Bryant <sean@cyberwang.net> To: Andrey Chernov <ache@freebsd.org>, Pawel Jakub Dawidek <pjd@FreeBSD.org>, src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/include stdio.h src/sys/sys unistd.h Message-ID: <4617D61A.5010104@cyberwang.net> In-Reply-To: <20070407170650.GA10631@nagual.pp.ru> References: <200704071602.l37G2V1c066806@repoman.freebsd.org> <20070407164022.GA10309@nagual.pp.ru> <20070407165435.GK63916@garage.freebsd.pl> <20070407170650.GA10631@nagual.pp.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4617D61A.5010104>