From nobody Thu Jan 4 11:54:54 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4T5Q4s3w3Yz56Cj3; Thu, 4 Jan 2024 11:55:09 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4T5Q4s04pvz54Yv; Thu, 4 Jan 2024 11:55:08 +0000 (UTC) (envelope-from kostikbel@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: from tom.home (kib@localhost [127.0.0.1] (may be forged)) by kib.kiev.ua (8.17.1/8.17.1) with ESMTP id 404BssBv046033; Thu, 4 Jan 2024 13:54:57 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 404BssBv046033 Received: (from kostik@localhost) by tom.home (8.17.1/8.17.1/Submit) id 404BssaJ046032; Thu, 4 Jan 2024 13:54:54 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 4 Jan 2024 13:54:54 +0200 From: Konstantin Belousov To: Olivier Certner Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: bd61c1e89dc4 - main - libthr: thr_attr.c: Clarity, whitespace and style Message-ID: References: <202401041044.404AiIN0046997@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202401041044.404AiIN0046997@gitrepo.freebsd.org> X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=4.0.0 X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-14) on tom.home X-Rspamd-Queue-Id: 4T5Q4s04pvz54Yv X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US] On Thu, Jan 04, 2024 at 10:44:18AM +0000, Olivier Certner wrote: > The branch main has been updated by olce: > > URL: https://cgit.FreeBSD.org/src/commit/?id=bd61c1e89dc4a40ba696de1785d423978e1c2147 > > commit bd61c1e89dc4a40ba696de1785d423978e1c2147 > Author: Olivier Certner > AuthorDate: 2023-11-24 16:00:53 +0000 > Commit: Olivier Certner > CommitDate: 2024-01-04 10:42:03 +0000 > > libthr: thr_attr.c: Clarity, whitespace and style > > Also, remove most comments, which don't add value. > > Reviewed by: emaste > Approved by: markj (mentor) > MFC after: 2 weeks > Sponsored by: The FreeBSD Foundation > Differential Revision: https://reviews.freebsd.org/D43005 > --- > lib/libthr/thread/thr_attr.c | 471 ++++++++++++++++++------------------------- > 1 file changed, 193 insertions(+), 278 deletions(-) > > diff --git a/lib/libthr/thread/thr_attr.c b/lib/libthr/thread/thr_attr.c > index 9740bed6ebd3..decbcb949167 100644 > --- a/lib/libthr/thread/thr_attr.c > +++ b/lib/libthr/thread/thr_attr.c > @@ -113,26 +113,15 @@ __weak_reference(_thr_attr_destroy, pthread_attr_destroy); > int > _thr_attr_destroy(pthread_attr_t *attr) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL) > - /* Invalid argument: */ > - ret = EINVAL; > - else { > - if ((*attr)->cpuset != NULL) > - free((*attr)->cpuset); > - /* Free the memory allocated to the attribute object: */ > - free(*attr); > + return (EINVAL); > > - /* > - * Leave the attribute pointer NULL now that the memory > - * has been freed: > - */ > - *attr = NULL; > - ret = 0; > - } > - return (ret); > + if ((*attr)->cpuset != NULL) The check is not needed. > + free((*attr)->cpuset); > + free(*attr); > + *attr = NULL; > + return (0); > } > > __weak_reference(_thr_attr_get_np, pthread_attr_get_np); > @@ -141,36 +130,44 @@ __weak_reference(_thr_attr_get_np, _pthread_attr_get_np); > int > _thr_attr_get_np(pthread_t pthread, pthread_attr_t *dstattr) > { > - struct pthread *curthread; > struct pthread_attr attr, *dst; > - int ret; > - size_t kern_size; > + struct pthread *curthread; > + size_t kern_size; > + int error; > > if (pthread == NULL || dstattr == NULL || (dst = *dstattr) == NULL) > return (EINVAL); > + > kern_size = _get_kern_cpuset_size(); > + > if (dst->cpuset == NULL) { > dst->cpuset = calloc(1, kern_size); > dst->cpusetsize = kern_size; > } > + > curthread = _get_curthread(); > - if ((ret = _thr_find_thread(curthread, pthread, /*include dead*/0)) != 0) > - return (ret); > + /* Arg 0 is to include dead threads. */ > + if ((error = _thr_find_thread(curthread, pthread, 0)) != 0) > + return (error); > + > attr = pthread->attr; > if (pthread->flags & THR_FLAGS_DETACHED) if ((pthread->flags & THR_FLAGS_DETACHED) != 0) there and in all similar places that test a flag bit > attr.flags |= PTHREAD_DETACHED; > - ret = cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, TID(pthread), > - dst->cpusetsize, dst->cpuset); > - if (ret == -1) > - ret = errno; > + > + error = cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, TID(pthread), > + dst->cpusetsize, dst->cpuset); > + if (error == -1) > + error = errno; > + > THR_THREAD_UNLOCK(curthread, pthread); > - if (ret == 0) { > - memcpy(&dst->pthread_attr_start_copy, > - &attr.pthread_attr_start_copy, > - offsetof(struct pthread_attr, pthread_attr_end_copy) - > - offsetof(struct pthread_attr, pthread_attr_start_copy)); > - } > - return (ret); > + > + if (error == 0) > + memcpy(&dst->pthread_attr_start_copy, > + &attr.pthread_attr_start_copy, > + offsetof(struct pthread_attr, pthread_attr_end_copy) - > + offsetof(struct pthread_attr, pthread_attr_start_copy)); > + > + return (0); > } > > __weak_reference(_thr_attr_getdetachstate, pthread_attr_getdetachstate); > @@ -179,22 +176,15 @@ __weak_reference(_thr_attr_getdetachstate, _pthread_attr_getdetachstate); > int > _thr_attr_getdetachstate(const pthread_attr_t *attr, int *detachstate) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL || detachstate == NULL) > - ret = EINVAL; > - else { > - /* Check if the detached flag is set: */ > - if ((*attr)->flags & PTHREAD_DETACHED) > - /* Return detached: */ > - *detachstate = PTHREAD_CREATE_DETACHED; > - else > - /* Return joinable: */ > - *detachstate = PTHREAD_CREATE_JOINABLE; > - ret = 0; > - } > - return (ret); > + return (EINVAL); > + > + if ((*attr)->flags & PTHREAD_DETACHED) For instance there. > + *detachstate = PTHREAD_CREATE_DETACHED; > + else > + *detachstate = PTHREAD_CREATE_JOINABLE; > + return (0); > } > > __weak_reference(_thr_attr_getguardsize, pthread_attr_getguardsize); > @@ -204,17 +194,12 @@ int > _thr_attr_getguardsize(const pthread_attr_t * __restrict attr, > size_t * __restrict guardsize) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL || guardsize == NULL) > - ret = EINVAL; > - else { > - /* Return the guard size: */ > - *guardsize = (*attr)->guardsize_attr; > - ret = 0; > - } > - return (ret); > + return (EINVAL); > + > + *guardsize = (*attr)->guardsize_attr; > + return (0); > } > > __weak_reference(_thr_attr_getinheritsched, pthread_attr_getinheritsched); > @@ -224,14 +209,12 @@ int > _thr_attr_getinheritsched(const pthread_attr_t * __restrict attr, > int * __restrict sched_inherit) > { > - int ret = 0; > > - if ((attr == NULL) || (*attr == NULL)) > - ret = EINVAL; > - else > - *sched_inherit = (*attr)->sched_inherit; > + if (attr == NULL || *attr == NULL) > + return (EINVAL); > > - return (ret); > + *sched_inherit = (*attr)->sched_inherit; > + return (0); > } > > __weak_reference(_thr_attr_getschedparam, pthread_attr_getschedparam); > @@ -241,14 +224,12 @@ int > _thr_attr_getschedparam(const pthread_attr_t * __restrict attr, > struct sched_param * __restrict param) > { > - int ret = 0; > > - if ((attr == NULL) || (*attr == NULL) || (param == NULL)) > - ret = EINVAL; > - else > - param->sched_priority = (*attr)->prio; > + if (attr == NULL || *attr == NULL || param == NULL) > + return (EINVAL); > > - return (ret); > + param->sched_priority = (*attr)->prio; > + return (0); > } > > __weak_reference(_thr_attr_getschedpolicy, pthread_attr_getschedpolicy); > @@ -258,14 +239,12 @@ int > _thr_attr_getschedpolicy(const pthread_attr_t * __restrict attr, > int * __restrict policy) > { > - int ret = 0; > > - if ((attr == NULL) || (*attr == NULL) || (policy == NULL)) > - ret = EINVAL; > - else > - *policy = (*attr)->sched_policy; > + if (attr == NULL || *attr == NULL || policy == NULL) > + return (EINVAL); > > - return (ret); > + *policy = (*attr)->sched_policy; > + return (0); > } > > __weak_reference(_thr_attr_getscope, pthread_attr_getscope); > @@ -275,17 +254,13 @@ int > _thr_attr_getscope(const pthread_attr_t * __restrict attr, > int * __restrict contentionscope) > { > - int ret = 0; > - > - if ((attr == NULL) || (*attr == NULL) || (contentionscope == NULL)) > - /* Return an invalid argument: */ > - ret = EINVAL; > > - else > - *contentionscope = (*attr)->flags & PTHREAD_SCOPE_SYSTEM ? > - PTHREAD_SCOPE_SYSTEM : PTHREAD_SCOPE_PROCESS; > + if (attr == NULL || *attr == NULL || contentionscope == NULL) > + return (EINVAL); > > - return (ret); > + *contentionscope = (*attr)->flags & PTHREAD_SCOPE_SYSTEM ? > + PTHREAD_SCOPE_SYSTEM : PTHREAD_SCOPE_PROCESS; > + return (0); > } > > __weak_reference(_pthread_attr_getstack, pthread_attr_getstack); > @@ -294,19 +269,14 @@ int > _pthread_attr_getstack(const pthread_attr_t * __restrict attr, > void ** __restrict stackaddr, size_t * __restrict stacksize) > { > - int ret; > - > - /* Check for invalid arguments: */ > - if (attr == NULL || *attr == NULL || stackaddr == NULL > - || stacksize == NULL ) > - ret = EINVAL; > - else { > - /* Return the stack address and size */ > - *stackaddr = (*attr)->stackaddr_attr; > - *stacksize = (*attr)->stacksize_attr; > - ret = 0; > - } > - return (ret); > + > + if (attr == NULL || *attr == NULL || stackaddr == NULL || > + stacksize == NULL) > + return (EINVAL); > + > + *stackaddr = (*attr)->stackaddr_attr; > + *stacksize = (*attr)->stacksize_attr; > + return (0); > } > > __weak_reference(_thr_attr_getstackaddr, pthread_attr_getstackaddr); > @@ -315,17 +285,12 @@ __weak_reference(_thr_attr_getstackaddr, _pthread_attr_getstackaddr); > int > _thr_attr_getstackaddr(const pthread_attr_t *attr, void **stackaddr) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL || stackaddr == NULL) > - ret = EINVAL; > - else { > - /* Return the stack address: */ > - *stackaddr = (*attr)->stackaddr_attr; > - ret = 0; > - } > - return (ret); > + return (EINVAL); > + > + *stackaddr = (*attr)->stackaddr_attr; > + return (0); > } > > __weak_reference(_thr_attr_getstacksize, pthread_attr_getstacksize); > @@ -335,17 +300,12 @@ int > _thr_attr_getstacksize(const pthread_attr_t * __restrict attr, > size_t * __restrict stacksize) > { > - int ret; > - > - /* Check for invalid arguments: */ > - if (attr == NULL || *attr == NULL || stacksize == NULL) > - ret = EINVAL; > - else { > - /* Return the stack size: */ > - *stacksize = (*attr)->stacksize_attr; > - ret = 0; > - } > - return (ret); > + > + if (attr == NULL || *attr == NULL || stacksize == NULL) > + return (EINVAL); > + > + *stacksize = (*attr)->stacksize_attr; > + return (0); > } > > __weak_reference(_thr_attr_init, pthread_attr_init); > @@ -354,40 +314,30 @@ __weak_reference(_thr_attr_init, _pthread_attr_init); > int > _thr_attr_init(pthread_attr_t *attr) > { > - int ret; > - pthread_attr_t pattr; > + pthread_attr_t pattr; > > _thr_check_init(); > > - /* Allocate memory for the attribute object: */ > - if ((pattr = (pthread_attr_t) malloc(sizeof(struct pthread_attr))) == NULL) > - /* Insufficient memory: */ > - ret = ENOMEM; > - else { > - /* Initialise the attribute object with the defaults: */ > - memcpy(pattr, &_pthread_attr_default, sizeof(struct pthread_attr)); > - > - /* Return a pointer to the attribute object: */ > - *attr = pattr; > - ret = 0; > - } > - return (ret); > + if ((pattr = malloc(sizeof(*pattr))) == NULL) > + return (ENOMEM); > + > + memcpy(pattr, &_pthread_attr_default, sizeof(struct pthread_attr)); Above you changed sizeeof(struct pthread_attr) to sizeof(*pattr), but there you left the type name. > + *attr = pattr; > + return (0); > } > > -__weak_reference(_pthread_attr_setcreatesuspend_np, pthread_attr_setcreatesuspend_np); > +__weak_reference(_pthread_attr_setcreatesuspend_np, \ > + pthread_attr_setcreatesuspend_np); > > int > _pthread_attr_setcreatesuspend_np(pthread_attr_t *attr) > { > - int ret; > > - if (attr == NULL || *attr == NULL) { > - ret = EINVAL; > - } else { > - (*attr)->suspend = THR_CREATE_SUSPENDED; > - ret = 0; > - } > - return (ret); > + if (attr == NULL || *attr == NULL) > + return (EINVAL); > + > + (*attr)->suspend = THR_CREATE_SUSPENDED; > + return (0); > } > > __weak_reference(_thr_attr_setdetachstate, pthread_attr_setdetachstate); > @@ -396,24 +346,17 @@ __weak_reference(_thr_attr_setdetachstate, _pthread_attr_setdetachstate); > int > _thr_attr_setdetachstate(pthread_attr_t *attr, int detachstate) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL || > (detachstate != PTHREAD_CREATE_DETACHED && > detachstate != PTHREAD_CREATE_JOINABLE)) > - ret = EINVAL; > - else { > - /* Check if detached state: */ > - if (detachstate == PTHREAD_CREATE_DETACHED) > - /* Set the detached flag: */ > - (*attr)->flags |= PTHREAD_DETACHED; > - else > - /* Reset the detached flag: */ > - (*attr)->flags &= ~PTHREAD_DETACHED; > - ret = 0; > - } > - return (ret); > + return (EINVAL); > + > + if (detachstate == PTHREAD_CREATE_DETACHED) > + (*attr)->flags |= PTHREAD_DETACHED; > + else > + (*attr)->flags &= ~PTHREAD_DETACHED; > + return (0); > } > > __weak_reference(_thr_attr_setguardsize, pthread_attr_setguardsize); > @@ -422,17 +365,12 @@ __weak_reference(_thr_attr_setguardsize, _pthread_attr_setguardsize); > int > _thr_attr_setguardsize(pthread_attr_t *attr, size_t guardsize) > { > - int ret; > > - /* Check for invalid arguments. */ > if (attr == NULL || *attr == NULL) > - ret = EINVAL; > - else { > - /* Save the stack size. */ > - (*attr)->guardsize_attr = guardsize; > - ret = 0; > - } > - return (ret); > + return (EINVAL); > + > + (*attr)->guardsize_attr = guardsize; > + return (0); > } > > __weak_reference(_thr_attr_setinheritsched, pthread_attr_setinheritsched); > @@ -441,17 +379,16 @@ __weak_reference(_thr_attr_setinheritsched, _pthread_attr_setinheritsched); > int > _thr_attr_setinheritsched(pthread_attr_t *attr, int sched_inherit) > { > - int ret = 0; > > - if ((attr == NULL) || (*attr == NULL)) > - ret = EINVAL; > - else if (sched_inherit != PTHREAD_INHERIT_SCHED && > - sched_inherit != PTHREAD_EXPLICIT_SCHED) > - ret = ENOTSUP; > - else > - (*attr)->sched_inherit = sched_inherit; > + if (attr == NULL || *attr == NULL) > + return (EINVAL); > + > + if (sched_inherit != PTHREAD_INHERIT_SCHED && > + sched_inherit != PTHREAD_EXPLICIT_SCHED) > + return (ENOTSUP); > > - return (ret); > + (*attr)->sched_inherit = sched_inherit; > + return (0); > } > > __weak_reference(_thr_attr_setschedparam, pthread_attr_setschedparam); > @@ -463,7 +400,7 @@ _thr_attr_setschedparam(pthread_attr_t * __restrict attr, > { > int policy; > > - if ((attr == NULL) || (*attr == NULL)) > + if (attr == NULL || *attr == NULL) > return (EINVAL); > > if (param == NULL) > @@ -474,7 +411,7 @@ _thr_attr_setschedparam(pthread_attr_t * __restrict attr, > if (policy == SCHED_FIFO || policy == SCHED_RR) { > if (param->sched_priority < _thr_priorities[policy-1].pri_min || > param->sched_priority > _thr_priorities[policy-1].pri_max) > - return (ENOTSUP); > + return (ENOTSUP); > } else { > /* > * Ignore it for SCHED_OTHER now, patches for glib ports > @@ -494,17 +431,15 @@ __weak_reference(_thr_attr_setschedpolicy, _pthread_attr_setschedpolicy); > int > _thr_attr_setschedpolicy(pthread_attr_t *attr, int policy) > { > - int ret = 0; > > - if ((attr == NULL) || (*attr == NULL)) > - ret = EINVAL; > - else if ((policy < SCHED_FIFO) || (policy > SCHED_RR)) { > - ret = ENOTSUP; > - } else { > - (*attr)->sched_policy = policy; > - (*attr)->prio = _thr_priorities[policy-1].pri_default; > - } > - return (ret); > + if (attr == NULL || *attr == NULL) > + return (EINVAL); > + if (policy < SCHED_FIFO || policy > SCHED_RR) > + return (ENOTSUP); > + > + (*attr)->sched_policy = policy; > + (*attr)->prio = _thr_priorities[policy-1].pri_default; > + return (0); > } > > __weak_reference(_thr_attr_setscope, pthread_attr_setscope); > @@ -513,41 +448,33 @@ __weak_reference(_thr_attr_setscope, _pthread_attr_setscope); > int > _thr_attr_setscope(pthread_attr_t *attr, int contentionscope) > { > - int ret = 0; > - > - if ((attr == NULL) || (*attr == NULL)) { > - /* Return an invalid argument: */ > - ret = EINVAL; > - } else if ((contentionscope != PTHREAD_SCOPE_PROCESS) && > - (contentionscope != PTHREAD_SCOPE_SYSTEM)) { > - ret = EINVAL; > - } else if (contentionscope == PTHREAD_SCOPE_SYSTEM) { > + > + if (attr == NULL || *attr == NULL || > + (contentionscope != PTHREAD_SCOPE_PROCESS && > + contentionscope != PTHREAD_SCOPE_SYSTEM)) > + return (EINVAL); > + > + if (contentionscope == PTHREAD_SCOPE_SYSTEM) > (*attr)->flags |= contentionscope; For me this looks somewhat confusing, in other places the explicit flag name symbol is used instead of a variable value. > - } else { > + else > (*attr)->flags &= ~PTHREAD_SCOPE_SYSTEM; > - } > - return (ret); > + return (0); > } > > __weak_reference(_pthread_attr_setstack, pthread_attr_setstack); > > int > _pthread_attr_setstack(pthread_attr_t *attr, void *stackaddr, > - size_t stacksize) > + size_t stacksize) > { > - int ret; > - > - /* Check for invalid arguments: */ > - if (attr == NULL || *attr == NULL || stackaddr == NULL > - || stacksize < PTHREAD_STACK_MIN) > - ret = EINVAL; > - else { > - /* Save the stack address and stack size */ > - (*attr)->stackaddr_attr = stackaddr; > - (*attr)->stacksize_attr = stacksize; > - ret = 0; > - } > - return (ret); > + > + if (attr == NULL || *attr == NULL || stackaddr == NULL || > + stacksize < PTHREAD_STACK_MIN) > + return (EINVAL); > + > + (*attr)->stackaddr_attr = stackaddr; > + (*attr)->stacksize_attr = stacksize; > + return (0); > } > > __weak_reference(_thr_attr_setstackaddr, pthread_attr_setstackaddr); > @@ -556,17 +483,12 @@ __weak_reference(_thr_attr_setstackaddr, _pthread_attr_setstackaddr); > int > _thr_attr_setstackaddr(pthread_attr_t *attr, void *stackaddr) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL || stackaddr == NULL) > - ret = EINVAL; > - else { > - /* Save the stack address: */ > - (*attr)->stackaddr_attr = stackaddr; > - ret = 0; > - } > - return(ret); > + return (EINVAL); > + > + (*attr)->stackaddr_attr = stackaddr; > + return (0); > } > > __weak_reference(_thr_attr_setstacksize, pthread_attr_setstacksize); > @@ -575,17 +497,12 @@ __weak_reference(_thr_attr_setstacksize, _pthread_attr_setstacksize); > int > _thr_attr_setstacksize(pthread_attr_t *attr, size_t stacksize) > { > - int ret; > > - /* Check for invalid arguments: */ > if (attr == NULL || *attr == NULL || stacksize < PTHREAD_STACK_MIN) > - ret = EINVAL; > - else { > - /* Save the stack size: */ > - (*attr)->stacksize_attr = stacksize; > - ret = 0; > - } > - return (ret); > + return (EINVAL); > + > + (*attr)->stacksize_attr = stacksize; > + return (0); > } > > static size_t > @@ -608,71 +525,69 @@ _get_kern_cpuset_size(void) > } > > __weak_reference(_pthread_attr_setaffinity_np, pthread_attr_setaffinity_np); > + > int > _pthread_attr_setaffinity_np(pthread_attr_t *pattr, size_t cpusetsize, > - const cpuset_t *cpusetp) > + const cpuset_t *cpusetp) > { > pthread_attr_t attr; > - int ret; > + size_t kern_size; > > if (pattr == NULL || (attr = (*pattr)) == NULL) > - ret = EINVAL; > - else { > - if (cpusetsize == 0 || cpusetp == NULL) { > - if (attr->cpuset != NULL) { > - free(attr->cpuset); > - attr->cpuset = NULL; > - attr->cpusetsize = 0; > - } > - return (0); > - } > - size_t kern_size = _get_kern_cpuset_size(); > - /* Kernel rejects small set, we check it here too. */ > - if (cpusetsize < kern_size) > - return (ERANGE); > - if (cpusetsize > kern_size) { > - /* Kernel checks invalid bits, we check it here too. */ > - size_t i; > - for (i = kern_size; i < cpusetsize; ++i) { > - if (((const char *)cpusetp)[i]) > - return (EINVAL); > - } > - } > - if (attr->cpuset == NULL) { > - attr->cpuset = calloc(1, kern_size); > - if (attr->cpuset == NULL) > - return (errno); > - attr->cpusetsize = kern_size; > + return (EINVAL); > + > + if (cpusetsize == 0 || cpusetp == NULL) { > + if (attr->cpuset != NULL) { > + free(attr->cpuset); > + attr->cpuset = NULL; > + attr->cpusetsize = 0; > } > - memcpy(attr->cpuset, cpusetp, kern_size); > - ret = 0; > + return (0); > + } > + > + kern_size = _get_kern_cpuset_size(); > + /* Kernel rejects small set, we check it here too. */ > + if (cpusetsize < kern_size) > + return (ERANGE); > + if (cpusetsize > kern_size) { > + /* Kernel checks invalid bits, we check it here too. */ > + size_t i; There should be a blank line after the local vars declaration. > + for (i = kern_size; i < cpusetsize; ++i) > + if (((const char *)cpusetp)[i] != 0) > + return (EINVAL); > + } > + if (attr->cpuset == NULL) { > + attr->cpuset = calloc(1, kern_size); What is the point of doing calloc() if the whole allocated memory is overwritten by the memcpy() call below? > + if (attr->cpuset == NULL) > + return (errno); > + attr->cpusetsize = kern_size; > } > - return (ret); > + memcpy(attr->cpuset, cpusetp, kern_size); > + return (0); > } > > __weak_reference(_pthread_attr_getaffinity_np, pthread_attr_getaffinity_np); > + > int > _pthread_attr_getaffinity_np(const pthread_attr_t *pattr, size_t cpusetsize, > - cpuset_t *cpusetp) > + cpuset_t *cpusetp) > { > pthread_attr_t attr; > - int ret = 0; > > if (pattr == NULL || (attr = (*pattr)) == NULL) > - ret = EINVAL; > - else { > - /* Kernel rejects small set, we check it here too. */ > - size_t kern_size = _get_kern_cpuset_size(); > - if (cpusetsize < kern_size) > - return (ERANGE); > - if (attr->cpuset != NULL) > - memcpy(cpusetp, attr->cpuset, MIN(cpusetsize, > - attr->cpusetsize)); > - else > - memset(cpusetp, -1, kern_size); > - if (cpusetsize > kern_size) > - memset(((char *)cpusetp) + kern_size, 0, > - cpusetsize - kern_size); > - } > - return (ret); > + return (EINVAL); > + > + /* Kernel rejects small set, we check it here too. */ > + size_t kern_size = _get_kern_cpuset_size(); > + if (cpusetsize < kern_size) > + return (ERANGE); > + if (attr->cpuset != NULL) > + memcpy(cpusetp, attr->cpuset, MIN(cpusetsize, > + attr->cpusetsize)); > + else > + memset(cpusetp, -1, kern_size); > + if (cpusetsize > kern_size) > + memset(((char *)cpusetp) + kern_size, 0, > + cpusetsize - kern_size); > + return (0); > }