This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [hurd,commited] hurd: Reimplement libc locks using mach's gsync


On Sun, 18 Mar 2018, Samuel Thibault wrote:

> diff --git a/hurd/hurdlock.c b/hurd/hurdlock.c

> +int __lll_abstimed_wait (void *ptr, int val,
> +  const struct timespec *tsp, int flags, int clk)

Lots of functions in this file have the same problem of the function 
return type being on the same line as the function name (the function name 
should always start a new line).

Other formatting issues in this file: you have operators at end of line in 
multi-line expressions rather than at start of line; continuation lines in 
expressions should line up immediately after the corresponding open 
parenthesis on a previous line, but this file has them less indented than 
that; don't use redundant parentheses around values passed to "return", 
it's not a function (so no "return (0)" or "return (ETIMEDOUT)" - but if 
it's a more complicated expression extending over more than one line, 
parentheses are appropriate to get the right indentation for the 
continuation lines).

> +  unsigned int val = atomic_load_relaxed((unsigned int *)ptr);

Missing space before '('.

> diff --git a/mach/lowlevellock.h b/mach/lowlevellock.h

> +/* Gsync flags.  */
> +#ifndef GSYNC_SHARED
> +  #define GSYNC_SHARED      0x01
> +  #define GSYNC_QUAD        0x02
> +  #define GSYNC_TIMED       0x04
> +  #define GSYNC_BROADCAST   0x08
> +  #define GSYNC_MUTATE      0x10
> +#endif

That's not how we do preprocessor indentation - we do "# define" with the 
"#" at start of line instead.

But actually nothing else defines GSYNC_SHARED and we discourage such uses 
of #ifndef as a coding practice anyway (better to have exactly one place 
that defines something, without using #ifndef, to be typo-proof).  So just 
remove the #ifndef and the indentation of the #defines here.

> diff --git a/manual/errno.texi b/manual/errno.texi
> index 73272fd884..8917cccb1e 100644
> --- a/manual/errno.texi
> +++ b/manual/errno.texi
> @@ -882,6 +882,16 @@ the normal result is for the operations affected to complete with this
>  error; @pxref{Cancel AIO Operations}.
>  @end deftypevr
>  
> +@deftypevr Macro int EOWNERDEAD
> +@standards{GNU, errno.h}
> +@errno{EOWNERDEAD, 120, Owner died}
> +@end deftypevr
> +
> +@deftypevr Macro int ENOTRECOVERABLE
> +@standards{GNU, errno.h}
> +@errno{ENOTRECOVERABLE, 121, State not recoverable}
> +@end deftypevr

In general I'd expect changes to errno.texi to be accompanied by 
regeneration of sysdeps/gnu/errlist.c.  Does such a regeneration result in 
no changes to the file, not even to the position in which these errno 
codes appear therein?

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]