[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