Re: [PATCH] add sched_getcpu

[Rich Felker <dalias@libc.org>]

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