From owner-freebsd-arch@FreeBSD.ORG Thu Apr 24 07:18:02 2008 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 33C371065672 for ; Thu, 24 Apr 2008 07:18:02 +0000 (UTC) (envelope-from scf@FreeBSD.org) Received: from mail.farley.org (farley.org [67.64.95.201]) by mx1.freebsd.org (Postfix) with ESMTP id EFF868FC20 for ; Thu, 24 Apr 2008 07:18:01 +0000 (UTC) (envelope-from scf@FreeBSD.org) Received: from thor.farley.org (HPooka@thor.farley.org [192.168.1.5]) by mail.farley.org (8.14.2/8.14.2) with ESMTP id m3O7HxaS071560; Thu, 24 Apr 2008 02:17:59 -0500 (CDT) (envelope-from scf@FreeBSD.org) Date: Thu, 24 Apr 2008 02:17:59 -0500 (CDT) From: "Sean C. Farley" To: Bruce Evans In-Reply-To: <20080424121743.P70536@delplex.bde.org> Message-ID: References: <20080424121743.P70536@delplex.bde.org> User-Agent: Alpine 1.10 (BSF 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; format=flowed; charset=US-ASCII X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.4 X-Spam-Checker-Version: SpamAssassin 3.2.4 (2008-01-01) on mail.farley.org Cc: freebsd-arch@FreeBSD.org Subject: Re: API type/include corrections X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Apr 2008 07:18:02 -0000 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 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 > when they would not be needed according to future > standards. OpenBSD fixed many syscall manpages so that the old POSIX > prerequisite of 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 . I think it was a mistake > even in 1998, since had enough namespace pollution > (including all of and much more) to work without any > explicit prequisites. However, it wasn't a bug in 1998, since > was still required for portability. FreeBSD cleaned up > most of the namespace pollution in 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 +#include + +#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