public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Joseph Myers <joseph@codesourcery.com>
To: Samuel Thibault <samuel.thibault@ens-lyon.org>
Cc: <libc-alpha@sourceware.org>, Agustina Arzille <avarzille@riseup.net>
Subject: Re: [hurd,commited] hurd: Reimplement libc locks using mach's gsync
Date: Mon, 19 Mar 2018 16:57:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.20.1803191644190.4537@digraph.polyomino.org.uk> (raw)
In-Reply-To: <20180318172408.19540-1-samuel.thibault@ens-lyon.org>

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

  reply	other threads:[~2018-03-19 16:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-18 17:24 Samuel Thibault
2018-03-19 16:57 ` Joseph Myers [this message]
2018-03-19 21:31   ` Joseph Myers
2018-03-20  1:59   ` Samuel Thibault

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1803191644190.4537@digraph.polyomino.org.uk \
    --to=joseph@codesourcery.com \
    --cc=avarzille@riseup.net \
    --cc=libc-alpha@sourceware.org \
    --cc=samuel.thibault@ens-lyon.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).