Re: git: bd61c1e89dc4 - main - libthr: thr_attr.c: Clarity, whitespace and style
Date: Thu, 04 Jan 2024 11:54:54 UTC
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 <olce.freebsd@certner.fr>
> AuthorDate: 2023-11-24 16:00:53 +0000
> Commit: Olivier Certner <olce@FreeBSD.org>
> 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);
> }