[OpenAFS-devel] File Descriptor use in the vol package

[Jeffrey Altman]

This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enigCC7A22C7ED699CBF10215433
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On 1/26/2011 11:45 AM, Simon Wilkinson wrote:
>=20
> On 26 Jan 2011, at 16:08, Rod Widdowson wrote:
>=20
>> The vol package uses three separate types of file descriptor.
>>
>> FILE *f
>> int fd
>> FD_t

There is one more component that Rod failed to describe.  On Windows,
file descriptors [int] (open) and FILE (fopen) are local to the C
Runtime Library (CRTL) instance.  It is not safe for a FILE or int
allocated in a DLL to be used by code an EXE that loads the DLL or in a
second DLL.  This is only an issue when the libraries and executables
are linked against different CRTL versions.  As distributed, this is not
a problem but when debugging it is often the case that a checked version
of a library will be mixed with a release build of an executable or vice
versa.

> I remain a little bit confused about what the problem is here, and what=

> the desired cure is. What follows is an attempt to clarify things ...
>=20
> My assumption is that having 3 different types of file descriptor isn't=

> a huge problem (it would be really nice to not have int fd, and FD_t,
> but we're always going to have 2, as long as we use both POSIX and
> native I/O).

The three different types is not a problem.  More types would not be a
problem either.

> It seems to me that the real issue is that in places we intermingle the=

> second and third I/O families. On Unix, this isn't a problem because
> these two families are identical. But on Windows, where we have our own=

> I/O calls in ntops.c, we'll (for example) open a file using "normal"
> open(), but then unlink it using nt_unlink(). This causes problems
> because we then don't get the unlink semantics that we desire.

The problem with unlink() is that open() and unlink() as implemented in
the CRTL was not done in a POSIX compliant manner.  There is nothing
"normal" about the CRTL implementations.  Secondly, the implementations
were designed back in Windows 3.x days and have never been modified by
Microsoft out of fear of breaking applications that depend on the
existing behaviors regardless of how poor they are.  The CRTL is meant
as a convenience to permit Unix apps to compile on Windows, they are not
meant to be used in any serious way in a Win32 application.  If POSIX
requirements are required with performance then the POSIX subsystem
should be used and not the Win32 subsystem.

On Windows, we are going to want to get away from using open() and
fopen() entirely in the long term.  There is no other way that we will
ever have a high performance file server.

The problem with mixing open() and nt_unlink() is not simply that an
'int' file descriptor is being passed to a Win32 API that requires a
HANDLE.  The problem is that once a file is opened on the system using
open() as long as that file descriptor is opened, the file cannot be
marked for deletion (or renamed.)  This is a property of the open mode
that is used by the open() implementation.

In other words, two sections of code can be completely consistent with
regards to their use of open() and nt_open() but if both APIs are used
to access the same file, the desired outcome cannot be achieved.

> So, there are two issues to resolve. The first is that we want to ensur=
e
> consistency of use. That is, we don't want to be explicitly (or
> implicitly) casting between FD_t and int. In fact, it would be highly
> desirable if the compiler stopped us from doing so.

Mixing FD_t and int is just wrong and this needs to be fixed no matter
what else is done.  The manual casts that are hiding the warnings on
Windows need to be removed.

Once Windows can build src/vol cleanly so that warnings can become
errors, then buildbot will point this out for us on the Windows builds.
 We don't necessarily need to implement an opaque type for all other
platforms.  Converting to an opaque type can be done at a later stage.

> Secondly, it would be really nice to not have 3 file descriptor types.
> This would entail replacing all use of the int family with the FD_t
> family. In my view, this would be best acheived by adding a platform
> independent I/O framework to our nascent opr portability library.

In my opinion we can perform an incremental improvement in src/vol today
and migrate it to opr in the future.  Rod's time to work on this project
is highly limited.  He is not going to be able to implement all of the
abstraction that you desire.

> Or is there more to this than I realise?

Jeffrey Altman


--------------enigCC7A22C7ED699CBF10215433
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)

iQIcBAEBAgAGBQJNQFXyAAoJEPd6c1WStpoEdfoQAKGPq3/w3KWcYkH9Jjzwv4Dw
Kv46pv+7tXr0iilN21aNULa+vG5devBxCiltrcaRAEp6VhP3gAYi8gQ/T/7hSgkI
mhIrEZEex/LK5rCM9Tn2kYTDLaBG+0f16sXb7wXR6jlfUWiHBkcAMtD9t6Gjy8YQ
gzMch+3XAk6feOWE9ttKxpxE/xKtv+40XXdNyHO2h4zZNKCsvcbI5Sp0LHiTAkRv
r7vaCMaQewKBBdiOcrDUR6IAL1vTJqe1b9rYcsY2d8IP1nVv9jjkgZvoXUPcUEYx
vt8AWK0oVkngn66EhRUJqQu5DG8iX1uEIWaq6kprUHS8oTGnzwB8PUutAYDbCn8B
EPOV+uIsA3v0V/dPInWQYuUvaxVo0Whv8BsjFanxdY0uF8lHinmxoOZ0ONv15W+s
oqwEC1z7Ug6PmZXH7l6jdhaBx0tit4/VpNKIUQsbDwxkuwuuOu4G8rMnU6Z2sHur
KvT2wvHHIiWuOn7iq6E0besz5iiQnSgnJiU9LgXdMugRGpz7dFkrSFnNTfi7PMco
e4q+1mjUkBrLLOaWZVnUc2b+PcWcPuVRsSHAW819mrCw5pOMQBZ2RyhLgttQt8aT
vcILG6a+nfykPEFGL0ThIv/xeQ91gMEsJN5Qpi5X225HlpOnEzlM4WuLy5pPOS1A
K212i84kypu0+Q+35jnK
=/Ow0
-----END PGP SIGNATURE-----

--------------enigCC7A22C7ED699CBF10215433--