API type/include corrections

Sean C. Farley scf at FreeBSD.org
Thu Apr 24 07:18:02 UTC 2008


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 at FreeBSD.org


More information about the freebsd-arch mailing list