[02/25] pthread_setaffinity (Linux variant): Rewrite to use VLA instead of alloca
Commit Message
On 03/02/2015 08:58 PM, Mike Frysinger wrote:
> On 01 Mar 2015 15:05, Florian Weimer wrote:
>> extend_alloca was used to emulate VLA deallocation.
>
> general note: i found the old code easier to read. i think GNU style requires
> newlines after decls and before the return statement. that would certainly help
> readability here.
What about this version? I think it makes the control flow more
explicit, and it prompted me to deal differently with the res == 0 case
(which was swept under the carpet before).
2015-03-16 Florian Weimer <fweimer@redhat.com>
* sysdeps/unix/sysv/linux/pthread_setaffinity.c
(__determine_cpumask_size): Replace extend_alloca with a
variable-length array. Always return zero if res == 0.
Comments
Florian Weimer <fweimer@redhat.com> writes:
> @@ -27,26 +26,29 @@
> size_t __kernel_cpumask_size attribute_hidden;
>
>
> -/* Determine the current affinity. As a side affect we learn
> - about the size of the cpumask_t in the kernel. */
> +/* Determine the size of cpumask_t in the kernel. */
> int
> __determine_cpumask_size (pid_t tid)
> {
> - INTERNAL_SYSCALL_DECL (err);
> - int res;
> -
> size_t psize = 128;
> - void *p = alloca (psize);
> -
> - while (res = INTERNAL_SYSCALL (sched_getaffinity, err, 3, tid, psize, p),
> - INTERNAL_SYSCALL_ERROR_P (res, err)
> - && INTERNAL_SYSCALL_ERRNO (res, err) == EINVAL)
> - p = extend_alloca (p, psize, 2 * psize);
> + int res;
>
> - if (res == 0 || INTERNAL_SYSCALL_ERROR_P (res, err))
> - return INTERNAL_SYSCALL_ERRNO (res, err);
> + for (psize = 128; ; psize *= 2)
The assignment is redundant.
Andreas.
On 03/16/2015 03:49 PM, Andreas Schwab wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>
>> @@ -27,26 +26,29 @@
>> size_t __kernel_cpumask_size attribute_hidden;
>>
>>
>> -/* Determine the current affinity. As a side affect we learn
>> - about the size of the cpumask_t in the kernel. */
>> +/* Determine the size of cpumask_t in the kernel. */
>> int
>> __determine_cpumask_size (pid_t tid)
>> {
>> - INTERNAL_SYSCALL_DECL (err);
>> - int res;
>> -
>> size_t psize = 128;
>> - void *p = alloca (psize);
>> -
>> - while (res = INTERNAL_SYSCALL (sched_getaffinity, err, 3, tid, psize, p),
>> - INTERNAL_SYSCALL_ERROR_P (res, err)
>> - && INTERNAL_SYSCALL_ERRNO (res, err) == EINVAL)
>> - p = extend_alloca (p, psize, 2 * psize);
>> + int res;
>>
>> - if (res == 0 || INTERNAL_SYSCALL_ERROR_P (res, err))
>> - return INTERNAL_SYSCALL_ERRNO (res, err);
>> + for (psize = 128; ; psize *= 2)
>
> The assignment is redundant.
It is; I'll drop it from the initialization.
On 16 Mar 2015 15:29, Florian Weimer wrote:
> + for (psize = 128; ; psize *= 2)
> + {
> + char buf[psize];
> + INTERNAL_SYSCALL_DECL (err);
style says there should be a blank line here -- after the decls and before the
first line of code (the res assignment). otherwise, this does look better than
v1. thanks!
-mike
On 03/16/2015 05:51 PM, Mike Frysinger wrote:
> On 16 Mar 2015 15:29, Florian Weimer wrote:
>> + for (psize = 128; ; psize *= 2) + { + char
>> buf[psize]; + INTERNAL_SYSCALL_DECL (err);
>
> style says there should be a blank line here -- after the decls and
> before the first line of code (the res assignment). otherwise,
> this does look better than v1. thanks!
Thanks, I've committed it after adding the blank line.
Could you have a look at the sister patch as well? Thanks.
<https://sourceware.org/ml/libc-alpha/2015-03/msg00544.html>
From 5dddb59c0c8ea06afebfee70026b854fbaf487f1 Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Sun, 1 Mar 2015 15:05:09 +0100
Subject: [PATCH] pthread_setaffinity (Linux variant): Rewrite to use VLA
instead of alloca
extend_alloca was used to emulate VLA deallocation. The new version
also handles the res == 0 corner case more explicitly, by returning 0
instead of the (potentially undefined, but usually zero) system call
error.
---
sysdeps/unix/sysv/linux/pthread_setaffinity.c | 32 ++++++++++++++-------------
1 file changed, 17 insertions(+), 15 deletions(-)
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <alloca.h>
#include <errno.h>
#include <pthreadP.h>
#include <sysdep.h>
@@ -27,26 +26,29 @@
size_t __kernel_cpumask_size attribute_hidden;
-/* Determine the current affinity. As a side affect we learn
- about the size of the cpumask_t in the kernel. */
+/* Determine the size of cpumask_t in the kernel. */
int
__determine_cpumask_size (pid_t tid)
{
- INTERNAL_SYSCALL_DECL (err);
- int res;
-
size_t psize = 128;
- void *p = alloca (psize);
-
- while (res = INTERNAL_SYSCALL (sched_getaffinity, err, 3, tid, psize, p),
- INTERNAL_SYSCALL_ERROR_P (res, err)
- && INTERNAL_SYSCALL_ERRNO (res, err) == EINVAL)
- p = extend_alloca (p, psize, 2 * psize);
+ int res;
- if (res == 0 || INTERNAL_SYSCALL_ERROR_P (res, err))
- return INTERNAL_SYSCALL_ERRNO (res, err);
+ for (psize = 128; ; psize *= 2)
+ {
+ char buf[psize];
+ INTERNAL_SYSCALL_DECL (err);
+ res = INTERNAL_SYSCALL (sched_getaffinity, err, 3, tid, psize, buf);
+ if (INTERNAL_SYSCALL_ERROR_P (res, err))
+ {
+ if (INTERNAL_SYSCALL_ERRNO (res, err) != EINVAL)
+ return INTERNAL_SYSCALL_ERRNO (res, err);
+ }
+ else
+ break;
+ }
- __kernel_cpumask_size = res;
+ if (res != 0)
+ __kernel_cpumask_size = res;
return 0;
}
--
2.1.0