This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add and use new glibc-internal futex API in sparc code.
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: David Miller <davem at davemloft dot net>, GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Tue, 9 Jun 2015 14:35:08 -0700 (PDT)
- Subject: Re: [PATCH] Add and use new glibc-internal futex API in sparc code.
- Authentication-results: sourceware.org; auth=none
- References: <1433800497 dot 21461 dot 455 dot camel at triegel dot csb> <20150609205152 dot A37C02C3BDC at topped-with-meat dot com> <1433884305 dot 10071 dot 27 dot camel at triegel dot csb>
> 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.
Thanks,
Roland