Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 22 Mar 2013 16:20:15 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Pawel Jakub Dawidek <pjd@freebsd.org>
Cc:        Konstantin Belousov <kostikbel@gmail.com>, freebsd-arch@freebsd.org
Subject:   Re: chflags(2)'s flags argument.
Message-ID:  <20130322155410.D953@besplex.bde.org>
In-Reply-To: <20130318210817.GA1367@garage.freebsd.pl>
References:  <20130317003559.GA1364@garage.freebsd.pl> <20130317064123.GM3794@kib.kiev.ua> <20130317111112.GC1364@garage.freebsd.pl> <20130317155743.GR3794@kib.kiev.ua> <20130317162021.GG1364@garage.freebsd.pl> <20130317162533.GT3794@kib.kiev.ua> <20130317164632.GI1364@garage.freebsd.pl> <20130317174205.GU3794@kib.kiev.ua> <20130317211027.GJ1364@garage.freebsd.pl> <20130318155059.V925@besplex.bde.org> <20130318210817.GA1367@garage.freebsd.pl>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 18 Mar 2013, Pawel Jakub Dawidek wrote:

> On Mon, Mar 18, 2013 at 04:51:22PM +1100, Bruce Evans wrote:
>> On Sun, 17 Mar 2013, Pawel Jakub Dawidek wrote:
>>> Maybe we could arrange ports build with lchflags(2) changed to take
>>> unsigned long to see how destructive the change really is, because my
>>> expectation is that not very.
>>
>> Why not use the correct type (u_int)?  u_long is just too long, since
>> st_flags is only 32 bits, so much larger and realler API and ABI changes
>> would be needed to support more than 32 flags.
>
> I'd prefer leave u_long, as it breaks API as little as possible. This is
> also the type that's being used by consumers of this API, as it was
> documented in chflags(2) since forever. Also, as I already mentioned,
> strtofflags(3) and fflagstostr(3) uses this type too.

When changing an API, it is best to change it in the correct direction.

>> The garbage also has some style bugs.  It spells "unsigned long" as
>> itself.  All other code in chflags.c still uses the normal abbreviation
>> u_long.
>
> Using u_long for chflags() prototypes in sys/stat.h will only work,
> because of this:
>
> #if !defined(_KERNEL) && __BSD_VISIBLE
> /*
> * XXX We get miscellaneous namespace pollution with this.
> */
> #include <sys/time.h>
> #endif
>
> As sys/time.h will include sys/types.h, eventhough sys/stat.h is trying
> to avoid that at the begining.

I wrote the comment.  mike@(gone) fixed the prototypes for *chflags()
so that they don't depend on the namespace pollution.  But sys/time.h
still has massive namespace pollution with many other dependences on
the pollution in sys/types.h.

Of coarse the header must not use the polluting spelling, but the
implementation can use any spelling it wants.

Actually the header should use the flags type __fflags_t.  That is
__uint32_t.  This is currently unusuable since the prototypes are
incompatible with it.  Changing the prototypes to it would fix them
in the way that I want.  Fixed-width types in APIs are design errors,
but we don't worry about that for mode_t, uid_t, ...  32-bit fixed
width types cause fewer problems than most since vax is the only
supported arch so ints are always 32 bits.

In glibc-2.6, chflags() takes type int for hurd and the default
chflags() (a stub that returns ENOSYS) also takes int.  So in ~20 years
when POSIX gets around to standardizing this, it would have to use a
typedefed type like fflags_t, and to support the FreeBSD mistakes it
would need different types for struct stat and the prototypes.

In linux-2.6.10, file flags are set by ioctl().  ioctl() in linux has
almost the same prototype as in BSD, so the arg type is unsigned long.
glibc-2.6 doesn't have any special support for this ioctl, and the
flags bits depend on the fs for both their name and their number, so
file flags are or were even harder to use in Linux than in FreeBSD,
and further from being standardizable.  glibc-2.6 has an fs-independent
bit in mount.h, with a warning that only newer systems support it.
linux-2.6.10 is too old to support it.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130322155410.D953>