[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 29 Feb 2016 13:38:16 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] add sched_getcpu
On Mon, Feb 29, 2016 at 08:23:33PM +0300, Alexander Monakov wrote:
> > +int sched_getcpu(void);
> > +#endif
> > int sched_getaffinity(pid_t, size_t, cpu_set_t *);
> > int sched_setaffinity(pid_t, size_t, const cpu_set_t *);
> >
> > diff --git a/src/sched/sched_getcpu.c b/src/sched/sched_getcpu.c
> > new file mode 100644
> > index 0000000..070d6e7
> > --- /dev/null
> > +++ b/src/sched/sched_getcpu.c
> > @@ -0,0 +1,10 @@
> > +#define _GNU_SOURCE
> > +#include <stdlib.h>
>
> Do you need this include?
>
> > +#include <sched.h>
>
> (this include could also be dropped; I think it's a matter of policy whether
> such includes are desirable or not, so please wait for comment from Rich)
Policy is to always include the header with the public declaration
(and any feature test macros necessary to get it) so that the compiler
checks the implementation against the public declaration.
> > +#include "syscall.h"
> > +
> > +int sched_getcpu(void) {
> > + int c, s;
> > + s = syscall(SYS_getcpu, &c, NULL, NULL);
> > + return (s == 0) ? c : s;
>
> This is wrong, as it doesn't set errno on error, and does not produce -1. This
> should be something like 'return s ? __syscall_ret(s) : c;' or maybe
> 'return __syscall_ret(s ? s : c);'.
Why is an error even possible here? Is it just to account for ancient
kernels that lack the syscall?
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
