Re: [PING] [PATCH v2] Resolve-flockfile-funlockfile-differences

[Florian Weimer <fweimer@redhat.com>]

* Cupertino Miranda via Libc-alpha:

> This is a request for review of the patch sent by Patrick McGehearty
> based on an inconsistency in flockfile and funlockfile definitions
> (macro/function) when used in glibc code.

It finally clicked to for me what is going on here.  Sorry for taking so
long.  It's a basic C problem.

_IO_funlockfile is defined as a macro and as a function.  The function
definition in stdio-common/funlockfile.c unlocks the stream
unconditionally.  The macro version checks if FSETLOCKING_BYCALLER is
active and skips the unlocking in that case.  Same for _IO_flockfile.

With &_IO_funlockfile, we get the unconditional unlock, but with
_IO_flockfile (S), we get the conditionalized lock.

The current code is clearly buggy.  It should be possible to write a
test for this, using ftrylockfile.  The direction of the change seems
right, too.

> Should both the macro/function definitions be coherent such that they should be
> used interchangeably in glibc code?

The external flockfile/funlockfile needs to be unconditional because I
think it is expected that these functions can be used to implement the
locking that is required if FSETLOCKING_BYCALLER is used.

Despite the recursive internal lock, checking for FSETLOCKING_BYCALLER
is probably more than a performance optimization because the unexpected
locking might introduce deadlocks.  So skipping the locking, as you did,
seems correct.

So I think we need the different internal and external behavior.  What
is clearly not okay though is that we use the same identifier for those
two different operations.  We cannot rename the external symbol, but we
really should rename the internal identifier.

Thanks,
Florian