From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6107 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads Date: Sat, 6 Sep 2014 14:52:46 -0400 Message-ID: <20140906185246.GW23797@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1410029588 6786 80.91.229.3 (6 Sep 2014 18:53:08 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 6 Sep 2014 18:53:08 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6120-gllmg-musl=m.gmane.org@lists.openwall.com Sat Sep 06 20:53:01 2014 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1XQL6Z-0004LU-H3 for gllmg-musl@plane.gmane.org; Sat, 06 Sep 2014 20:52:59 +0200 Original-Received: (qmail 3804 invoked by uid 550); 6 Sep 2014 18:52:58 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 3793 invoked from network); 6 Sep 2014 18:52:58 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6107 Archived-At: On Mon, Sep 01, 2014 at 12:46:23AM +0200, Jens Gustedt wrote: > The intent of this is to avoid name space pollution of the C threads > implementation. I found a few bugs in this which I'm fixing and about to commit if all goes well: > diff --git a/src/mman/mprotect.c b/src/mman/mprotect.c > index f488486..535787b 100644 > --- a/src/mman/mprotect.c > +++ b/src/mman/mprotect.c > @@ -2,10 +2,12 @@ > #include "libc.h" > #include "syscall.h" > > -int mprotect(void *addr, size_t len, int prot) > +int __mprotect(void *addr, size_t len, int prot) > { > size_t start, end; > start = (size_t)addr & -PAGE_SIZE; > end = (size_t)((char *)addr + len + PAGE_SIZE-1) & -PAGE_SIZE; > return syscall(SYS_mprotect, start, end-start, prot); > } > + > +weak_alias(__mprotect, mprotect); I tested and inlining the syscall does not make pthread_create.o smaller. It would eliminate a dependency on one extra file, so I might make the change later, but for now I'm just doing it your way in the interest of getting things committed without major edits to the patches. > diff --git a/src/thread/__timedwait.c b/src/thread/__timedwait.c > index d6f1233..3d62272 100644 > --- a/src/thread/__timedwait.c > +++ b/src/thread/__timedwait.c > @@ -4,6 +4,9 @@ > #include "futex.h" > #include "syscall.h" > > +int __pthread_setcancelstate(int new, int *old); > +int __clock_gettime(clockid_t clk, struct timespec *ts); I've removed arg names from prototypes like this just for consistency with how it's done elsewhere. > @@ -139,7 +143,7 @@ static void init_file_lock(FILE *f) > > void *__copy_tls(unsigned char *); > > -int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg) > +static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg) This static is wrong if the goal is to go ahead and get the files in the form where they're "ready to use" for C11. I saw it was reverted in the later patch, but I'm just taking it out here. > diff --git a/src/thread/pthread_getspecific.c b/src/thread/pthread_getspecific.c > index b2a282c..bfc4294 100644 > --- a/src/thread/pthread_getspecific.c > +++ b/src/thread/pthread_getspecific.c > @@ -1,7 +1,10 @@ > #include "pthread_impl.h" > > -void *pthread_getspecific(pthread_key_t k) > +static void *__pthread_getspecific(pthread_key_t k) > { > struct pthread *self = __pthread_self(); > return self->tsd[k]; > } > + > +weak_alias(__pthread_getspecific, pthread_getspecific); > +weak_alias(__pthread_getspecific, tss_get); This actually goes ahead and implements one C11 function, which is not horrible, but mildly inconsistent. I think I'll move it to the later patch that's actually supposed to implement these functions. > diff --git a/src/thread/pthread_mutex_trylock.c b/src/thread/pthread_mutex_trylock.c > index e851517..9be7930 100644 > --- a/src/thread/pthread_mutex_trylock.c > +++ b/src/thread/pthread_mutex_trylock.c > @@ -50,9 +50,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m) > return 0; > } > > -int pthread_mutex_trylock(pthread_mutex_t *m) > +static int __pthread_mutex_trylock(pthread_mutex_t *m) > { > if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL) > return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY; > return __pthread_mutex_trylock_owner(m); > } > + > +weak_alias(__pthread_mutex_trylock, pthread_mutex_trylock); This static looks like an actual bug that would break the patch series as a whole; I can't see where it's made visible for mtx_trylock to use later. Removing static here. Rich