Date: Thu, 24 Apr 2008 02:17:59 -0500 (CDT) From: "Sean C. Farley" <scf@FreeBSD.org> To: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-arch@FreeBSD.org Subject: Re: API type/include corrections Message-ID: <alpine.BSF.1.10.0804240102060.33196@thor.farley.org> In-Reply-To: <20080424121743.P70536@delplex.bde.org> References: <alpine.BSF.1.10.0804232000490.29423@thor.farley.org> <20080424121743.P70536@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 24 Apr 2008, Bruce Evans wrote:
> On Wed, 23 Apr 2008, Sean C. Farley wrote:
>
>> I am going through a list where I track items I wish to fix as I run
>> across them. I just like to make sure they are actually broken
>> before fixing them. :)
>>
>> The items in question:
>> 1. Should the man page for mkdir(2) include sys/types.h? Open Group
>> docs[1] do not have it, yet they do use it in the example. It is
>> not needed to compile their example.
>
> No.
>
> POSIX required <sys/types.h> for most syscalls including mkdir() until
> 2001. 4.4 and earlier BSDs were very inconsistent about the necessary
> includes. They generally give a prototype and the include that
> declares the prototype, but are sloppy about the includes that a
> prerequisites for the primary include. I fixed many syscall manpages
> so that the example obtained by turning the include list and the
> declaration into a program compiles, but only searched for examples
> that didn't compile and tried not to add prerequisites like
> <sys/types.h> when they would not be needed according to future
> standards. OpenBSD fixed many syscall manpages so that the old POSIX
> prerequisite of <sys/types.h> is given explicitly, and FreeBSD
> imported this fix in a few manpages, in particular in mkdir.2 in 1998.
> This fix became a bug in 2001 or earlier when POSIX obsoleted
> everything having to include <sys/types.h>. I think it was a mistake
> even in 1998, since <sys/stat.h> had enough namespace pollution
> (including all of <sys/types.h> and much more) to work without any
> explicit prequisites. However, it wasn't a bug in 1998, since
> <sys/types.h> was still required for portability. FreeBSD cleaned up
> most of the namespace pollution in <sys/stat.h> and some other headers
> in 2001-2003, so the change to mkdir.2 in 1998 has been completely
> bogus for more than 5 years and the 15+ year old include list is
> correct again :-).
Thank you; that explains a lot. I am beginning to grasp the convoluted
history involved in these ancient API calls. :)
I will fix the man page by removing that prerequisite.
>> 2. Should readpassphrase(3) include sys/types.h in the man page, or
>> should it be added to readpassphrase.h? It is needed to compile
>> even a simple program to pick up size_t.
>
> Neither. readpassphrase.h should declare everything that it uses but no
> more. Since readpassphrase.h was born broken, the man page should have
> documented prerequisites that actuallyt work.
Is the following appropriate, or should the man page be the one changed?
That was basically my original question. From your comment, I see that
the header should have included sys/_types.h instead of sys/types.h and
defined size_t itself. Is that correct?
-----------------------------------
--- readpassphrase.h 8 Mar 2002 20:52:52 -0000 1.2
+++ readpassphrase.h 24 Apr 2008 06:07:21 -0000
@@ -39,6 +39,12 @@
#define RPP_SEVENBIT 0x10 /* Strip the high bit from input. */
#include <sys/cdefs.h>
+#include <sys/_types.h>
+
+#ifndef _SIZE_T_DECLARED
+typedef __size_t size_t;
+#define _SIZE_T_DECLARED
+#endif
__BEGIN_DECLS
char * readpassphrase(const char *, char *, size_t, int);
-----------------------------------
>> 3. Should chflags(2), lchflags(2) and fchflags(2) have the flags
>> argument be of type fflags_t (uint32_t) instead of u_long? The
>> man page says u_long while the type for st_flags in struct stat is
>> fflags_t.
>
> The APIs and ABIs for this are broken and depend on bugs to work, so
> fixing them risks disturbing the bugs. fflags_t is the result of
> incomplete fixes. Now the brokenness is as follows:
> - these syscalls never really took anything except an int arg, since
> syscalls.master has always said that the arg is an int and the
> kernel parts of the syscalls has always copied this arg to "int
> flags".
> - chflags(2) and fchflags(2) originally took a u_long arg. Now they
> take an fflags_t arg. Binary magic results in these
> incompatibilities having little effect.
> - lchflags(2) takes an int arg, because it was blindly imported from
> OtherBSD where this bug suite had apparently been fixed differently
> and completely by changing all the types to int. The others had
> already been changed to take an fflags_t arg in FreeBSD, but only at
> the library level. This works without so much binary magic.
> - struct stat now uses fflags_t st_flags.
> - vnode.h has always used u_long va_flags.
> So flags usually start as a an fflags_t which is usually a uint32_t,
> then are converted to an int which is usually an int32_t, then are
> converted to a u_long which is often larger than a uint32_t, then are
> converted to an fflags_t which is usually a uint32_t.
I feel converted. :) Since fflags_t is usually (or is it actually
always) a uint32_t, then the following would need to be done:
- Syscalls changed to accept fflags_t (uint32_t)
- Symbol.map addition to reflect new version of ABI
- sys/stat.h change to *flags() calls to accept fflags_t
- sys/vnode.h change from u_long to fflags_t
With a casual glance in the kernel, files needing changes or scrutiny
with this change:
sys/fs/cd9660/cd9660_vnops.c
sys/fs/coda/coda.h
sys/kern/vfs_vnops.c (The u_long to uint32_t conversion you
mentioned? sb->st_flags = vap->va_flags)
sys/kern/vfs_syscalls.c
sys/ufs/ufs/ufs_vnops.c
- I am probably forgetting with my limited experience in the kernel.
- I almost forgot the change to the man page, which is where I noticed
this first. /me smacks forehead :)
Sean
--
scf@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.1.10.0804240102060.33196>