Re: [PATCH] New condvar implementation that provides stronger ordering guarantees.

[Florian Weimer <fweimer at redhat dot com>]

On 06/14/2016 08:15 PM, Torvald Riegel wrote:

> > Or alternatively, why is the s variable not reloaded?
>
> It is.  C11's compare_exchange updates the expected value if the it the
> comparison failed (that's why we pass in the address of s).

Ahh.  This explains it.

> > I haven't really reviewed this patch.
>
> Thanks for taking a look nonetheless.
>
> > One small nit: GNU style does not
> > use a parenthesis at the end of a line, as in (among others):
> >
> > +  uint64_t r = __condvar_fetch_add_64_relaxed (
> >
> > +  return __condvar_load_64_relaxed (
>
> What's the expected solution, parenthesis on the next line?

Exactly, like this:

   return long_function name
     (some_long expression_on_the_next_line_in_the_input_file);

> > There are some magic numbers 2/4/8, which might better use symbolic
> > constants.
>
> I considered this, but kind of preferred the short numbers.  If we
> prefer constants with names, I'll change that.

I prefer them in the code I'm working on.  But I understand that 
preferences differ.

> > pthread_cond_common.c should be a header file (pthread_cond_common.h).
>
> There are many cases of C files being included across all of glibc.  Are
> there precise rules for when to pick .h vs. .c?  pthread_cond_common.c
> doesn't have just declarations or small helper functions, so .c seemd to
> be the right choice.

As far as I can tell, we use .c only if the file is *also* compiled as a 
C source file.  In such cases, there are some preprocessor macros you 
can define to alter the behavior, before including the file.  In other 
cases, it's used to create test cases with different compiler/linker 
flags (e.g., static linking).  Or it's an override for the sysdeps 
mechanism.

Thanks,
Florian