From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 63861 invoked by alias); 25 Jan 2018 16:12:57 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 63851 invoked by uid 89); 25 Jan 2018 16:12:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=arrangements, achieving, ceased, credit X-HELO: relay1.mentorg.com Date: Thu, 25 Jan 2018 16:12:00 -0000 From: Joseph Myers To: Samuel Thibault CC: Thomas Schwinge , Florian Weimer , GNU C Library , , David Michael Subject: Re: Upstreaming the glibc Hurd port In-Reply-To: <20180125014143.2hxhzon5lzxtqq6j@var.youpi.perso.aquilenet.fr> Message-ID: References: <20180118154251.ynfyugkmog7kujom@var.youpi.perso.aquilenet.fr> <20180118165923.ymreisuzexxz4gt3@var.youpi.perso.aquilenet.fr> <20180118235924.r4z4ppvj7xlvmmfp@var.youpi.perso.aquilenet.fr> <87a7xaupjx.fsf@euler.schwinge.homeip.net> <20180124011051.5s2vugyq3ybnurwc@var.youpi.perso.aquilenet.fr> <20180124012726.tibylwp4re5dtgc3@var.youpi.perso.aquilenet.fr> <20180125014143.2hxhzon5lzxtqq6j@var.youpi.perso.aquilenet.fr> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-SW-Source: 2018-01/txt/msg00814.txt.bz2 Regarding the libpthread / htl code, and getting it ready for inclusion in glibc master: Obviously my general coding style comments at 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 ). * 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