From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10473 Path: news.gmane.org!.POSTED!not-for-mail From: Alexander Monakov Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] add pthread_setname_np Date: Fri, 16 Sep 2016 12:03:16 +0300 (MSK) Message-ID: References: <20160916002351.GA5913@nyan> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Trace: blaine.gmane.org 1474016658 20073 195.159.176.226 (16 Sep 2016 09:04:18 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 16 Sep 2016 09:04:18 +0000 (UTC) User-Agent: Alpine 2.20.13 (LNX 116 2015-12-14) To: musl@lists.openwall.com Original-X-From: musl-return-10486-gllmg-musl=m.gmane.org@lists.openwall.com Fri Sep 16 11:04:14 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1bkp3x-0003NW-Cg for gllmg-musl@m.gmane.org; Fri, 16 Sep 2016 11:04:01 +0200 Original-Received: (qmail 32400 invoked by uid 550); 16 Sep 2016 09:04:01 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 32382 invoked from network); 16 Sep 2016 09:04:00 -0000 In-Reply-To: <20160916002351.GA5913@nyan> Xref: news.gmane.org gmane.linux.lib.musl.general:10473 Archived-At: Hi, On Thu, 15 Sep 2016, Felix Janda wrote: > --- /dev/null > +++ b/src/thread/pthread_setname_np.c > @@ -0,0 +1,25 @@ > +#include > +#include > +#include > +#include > + > +#include "pthread_impl.h" This should bring in the declaration of pthread_setname_np from pthread.h by #defin'ing _GNU_SOURCE prior to inclusion. (today, this rule is not enforced with warnings and thus not always followed in musl, for example the recent pthread_tryjoin_np patch missed that as well) > +int pthread_setname_np(pthread_t thread, const char *name) > +{ > + int fd, cs, status = 0; > + char f[sizeof "/proc/self/task//comm" + 3*sizeof(int)]; > + size_t len; > + > + if ((len = strnlen(name, 16)) > 15) return ERANGE; > + > + if (thread == pthread_self()) This likely should use the static inline function __pthread_self()? > + return prctl(PR_SET_NAME, (unsigned long)name, 0, 0, 0) ? errno : 0; First, prctl is declared as a variadic function, so zeros should be passed with the right type (0ul); passing fewer than 5 arguments will cause musl's implementation of prctl to invoke UB, since it always retrieves 4 variadic arguments. Second, while I don't have a strong opinion on whether musl wants to use prctl here (my previous comment in this thread was based on the wrong idea that prctl would be sufficient in all cases - sorry about that), I think some tuning would be nice in case the decision is to use prctl. So, either return -__syscall(SYS_prctl, PR_SET_NAME, name); (not sure if Rich would like it), or if (thread == pthread_self()) { status = prctl(...); } else { ... fd = status = open(f, O_WRONLY); if (fd >= 0) { status = write(fd, name, len); close(fd); } pthread_setcancelstate(cs, 0); } return status ? errno : 0; > + snprintf(f, sizeof f, "/proc/self/task/%d/comm", thread->tid); > + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); > + if ((fd = open(f, O_WRONLY)) < 0 || write(fd, name, len) < 0) status = errno; > + if (fd >= 0) close(fd); > + pthread_setcancelstate(cs, 0); > + return status; > +} Thanks. Alexander