[PATCH] Add and use new glibc-internal futex API in sparc code.

Torvald Riegel triegel@redhat.com
Wed Jun 10 18:58:00 GMT 2015


On Tue, 2015-06-09 at 14:35 -0700, Roland McGrath wrote:
> > On Tue, 2015-06-09 at 13:51 -0700, Roland McGrath wrote:
> > > Arguably every use of ignore_value should have a comment explaining why
> > > it's safe to elide the error checking.
> > 
> > That's always the same pattern.  Would it be okay for you if I just note
> > that pattern in the futex_wait comments?  
> 
> That is certainly good to do, but I think we still want at least a brief
> comment for every use of ignore_value where it's not totally obvious (most
> existing uses are in stubs where it's clear).  A detailed comment about the
> general case in one place means that the individual comments can just be 
> very terse like /* futex_waits errors ignored since we'll retry.  */
> 
> But if it's a commonly-repeated case, then it probably merits a wrapper
> macro that constitutes self-documentation.  i.e., define a
> "futex_wait_ignore_error" or "futex_wait_noerror" macro to use in all these
> places, and then the comment on the macro definition can explain where it's
> appropriate to use and why.  Or perhaps name the macro "futex_wait_in_loop"
> or something better that more obviously says in the name where it's
> appropriate to use rather than just what is concretely does.
> 
> The upshot is that avoiding error checking is always unusual and a casual
> reader of just a little bit of code (especially a reviewer seeing just the
> patch context) should not have to hunt around to convince himself that it
> is safe.  If it's a macro that makes its uses self-documenting, then that
> does it.  If it's a terse comment that points clearly to where fuller
> rationale can be read, then that does it too.

I've added a futex_wait_simple call that returns void and explains the
use case.  See my updated patch for the futex API.  Attached is the
updated sparc code for completeness.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: futex-sparc.patch
Type: text/x-patch
Size: 10769 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20150610/04921268/attachment.bin>


More information about the Libc-alpha mailing list