Re: Upstreaming the glibc Hurd port

[Joseph Myers <joseph@codesourcery.com>]

Regarding the libpthread / htl code, and getting it ready for inclusion in 
glibc master:

Obviously my general coding style comments at 
<https://sourceware.org/ml/libc-alpha/2018-01/msg00646.html> apply equally 
to this code.  Apart from that:

* Remove htl/ChangeLog.  We don't have subdirectory ChangeLogs now.  
Everything goes in the top-level ChangeLog, with older ChangeLogs (that 
have actual ChangeLog content) being in the ChangeLog.old/ directory.  
When importing files / patches previously maintained elsewhere, I *do* 
think the ChangeLog header for those patches should name all authors who 
contributed to them (which may make for a long header with lots of authors 
listed).  (If the GNU Coding Standards change to stop requiring ChangeLog 
format and we decide to stop using it, we'll still need some convention 
for how to credit multi-author patches.)

* Remove htl/tests/.cvsignore.

* Support for building htl outside the glibc source tree (or with older 
glibc versions) should not be included.

* Files in htl/sysdeps should be moved into appropriate locations in the 
main sysdeps tree, just as was done with nptl/sysdeps when nptl ceased to 
be an add-on.

* You seem to have some custom system for building tests in htl/tests.  
Tests should be built and run using the normal glibc testsuite machinery 
(and should use <support/test-driver.c>).

* You have four installed non-bits/ API headers: pthread.h 
pthread/pthread.h pthread/pthreadtypes.h semaphore.h.  NPTL has just 
pthread.h and semaphore.h.  Do you really need pthread/pthread.h and 
pthread/pthreadtypes.h as installed public API headers?  I'd expect the 
same two API headers as NPTL has.

* The bits/ convention is for headers that are (a) installed, (b) only for 
use by other installed headers, not for direct inclusion by users (and 
sometimes have #error conditionals to guard against direct inclusion).  
Within that convention, there's a newer bits/types/ convention for headers 
defining a single type, which is intended as a replacement for the older 
__need_* convention.  Now, this brings up three points regarding the htl 
code (and possibly other pieces in the Hurd port).

(i) You still have some headers that define or use __need_*; those should 
be changed to use bits/types/ headers (__need_* has generally been 
eliminated from glibc, except where it's defined before including *GCC* 
headers, not glibc ones).

(ii) Some of the bits/ headers in htl look like they are just defining a 
single type, so should actually be bits/types/ headers named according to 
the new convention.

(iii) bits/memory.h and bits/pt-atomic.h don't appear to be installed 
headers, meaning they should not have bits/ names.  Actually, uses of 
those headers should probably be changed to use glibc's existing atomic.h 
interfaces, and those htl-specific headers removed - if for some reason 
the existing atomic.h interfaces are insufficient, maybe those interfaces 
need to be extended.


The following would be desirable cleanups, but maybe for after the code is 
in master:

* I'd expect that NPTL tests (and likewise HTL tests) are largely tests of 
POSIX (or GNU extension) threading functionality, not of features specific 
to one threading implementation.  Thus, they should as far as possible be 
shared between the different threading implementations.  I don't know the 
best directory arrangements for achieving that.

* Hopefully C11 threads support will be added for 2.28 (didn't get 
reviewed in time for 2.27).  The existing patches put it in the nptl/ 
directory, but again, to the extent that it actually builds on generic 
pthreads functionality, the code and its tests should be shared as much as 
possible between NPTL and HTL.

* Likewise for the pthread.h and semaphore.h API headers - only parts 
genuinely specific to a given implementation should be split out into 
implementation-specific bits/ headers.  (Indeed, the NPTL semaphore.h is 
already in sysdeps/pthread/ not an NPTL-specific directory - do you really 
need a different one for HTL?)

-- 
Joseph S. Myers
joseph@codesourcery.com