use of unsafe stdio _unlocked variants in multithreaded sort

[Pádraig Brady]

While looking at the recent possible gettext() multithreaded issue,
I also noticed that there were references to *_unlocked functions
in the sort command, which doesn't look right.

$ nm src/sort.o | grep unlocked
    U clearerr_unlocked
    U fflush_unlocked
    U fputs_unlocked
    U fread_unlocked
    U fwrite_unlocked
$ nm src/sort | grep unlocked
    U clearerr_unlocked@@GLIBC_2.2.5
    U fflush_unlocked@@GLIBC_2.2.5
    U fputs_unlocked@@GLIBC_2.2.5
    U fread_unlocked@@GLIBC_2.2.5
    U fwrite_unlocked@@GLIBC_2.2.5

I can easily remove most with the simple attached patch:

$ nm src/sort.o | grep unlocked
$ nm src/sort | grep unlocked
    U fputs_unlocked@@GLIBC_2.2.5
    U fread_unlocked@@GLIBC_2.2.5

So the remaining must be coming from gnulib:

$ (cd ~/git/gnulib/lib/ && grep unlocked-io.h *)
argmatch.c:# include "unlocked-io.h"
close-stream.c:# include "unlocked-io.h"
error.c:# include "unlocked-io.h"
exclude.c:# include "unlocked-io.h"
getdelim.c:# include "unlocked-io.h"
getndelim2.c:# include "unlocked-io.h"
getpass.c:#  include "unlocked-io.h"
getusershell.c:# include "unlocked-io.h"
linebuffer.c:# include "unlocked-io.h"
md2.c:# include "unlocked-io.h"
md4.c:# include "unlocked-io.h"
md5.c:# include "unlocked-io.h"
mountlist.c:# include "unlocked-io.h"
posixtm.c:# include "unlocked-io.h"
readtokens.c:# include "unlocked-io.h"
readutmp.c:# include "unlocked-io.h"
sha1.c:# include "unlocked-io.h"
sha256.c:# include "unlocked-io.h"
sha512.c:# include "unlocked-io.h"
version-etc.c:# include "unlocked-io.h"

I suppose the most general solution would be to
have a separate libcoreutils.a generated with
USE_UNLOCKED_IO set to 0 for linking by multithreaded
programs in a project. That seems overkill in this case though,
as most of those uses really don't care about performance
and could probably just use the standard functions?

So the ones that sort(1) potentially uses are as follows,
with the last two possibly dealing with large amounts of data
and so would be best to keep using unlocked routines if possible.
Now sort actually uses those two before it creates threads,
so that's not a problem.

argmatch.c:# include "unlocked-io.h"
close-stream.c:# include "unlocked-io.h"
error.c:# include "unlocked-io.h"
version-etc.c:# include "unlocked-io.h"
md5.c:# include "unlocked-io.h"
readtokens.c:# include "unlocked-io.h"

So the only two gnulib modules that sort(1) uses from
separate threads, and the only ones that would need to
be adjusted for sort(1) at least are:

close-stream.c:# include "unlocked-io.h"
error.c:# include "unlocked-io.h"

But I'd probably remove unlocked-io.h use from
a few more of the top gnulib module list while at it?

I wonder should we add a warning if USE_UNLOCKED_IO is 1
while any of the gnulib threading modules are included?

Also it's worth noting that since we'd still have
references to unlocked functions from single threaded
parts of sort(1), it would be awkward to audit that
going forward. Any ideas to ease the maintenance burden
there would be appreciated.

thanks,
Pádraig.

sort-unlocked.patch
Description: Text Data