| Summary: | Confusing documentation for apr_pool_cleanup_run() | ||
|---|---|---|---|
| Product: | APR | Reporter: | Jonathan Wakely <zilla> |
| Component: | APR | Assignee: | Apache Portable Runtime bugs mailinglist <bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | Keywords: | PatchAvailable |
| Priority: | P2 | ||
| Version: | HEAD | ||
| Target Milestone: | --- | ||
| Hardware: | Other | ||
| OS: | other | ||
| Attachments: |
Fix confusing docs
Clarify docs for cleanup functions Clarify docs for cleanup functions |
||
|
Description
Jonathan Wakely
2005-08-02 18:34:08 UTC
Created attachment 15853 [details]
Fix confusing docs
This patch removes the part I think is confusing, and corrects two instances of
"The pool register the cleanup with" changing it to "The pool to register the
cleanup with"
I feel a need to go through the docs for the various apr_pool_cleanup_xxx() functions and add more descriptive docs, explaining that the combination of function pointer, data and pool defines "a cleanup" and that the same combination must be used to unregister a cleanup. Before I waste my time, would such a patch be accepted? We absolutely entertain docs patches, as well as code patches. Please feel free to proceed. Should we keep this incident open and commit your entire set of docs patches at once? Thanks for the quick response. I'll try to submit a further patch today or tomorrow, so if you want to wait for that then I'll use this bug to collect them all. Created attachment 16014 [details]
Clarify docs for cleanup functions
(Sorry for the delay, I was on holiday.)
This patch adds a Doxygen group for the cleanup functions, fixes some typos and
IMHO clarifies the behaviour of some of the cleanup functions.
For the functions that unregister a cleanup I have said:
The most recently registered cleanup with the same values of @a p,
@a data and @a cleanup will be removed.
That description doesn't explicitly say that the cleanup is registered with
pool 'p' but I think saying "the same values of p, data and cleanup" is
accurate enough but also short and pithy. Would it be better to say something
like this?
Removes the last cleanup registered with @a p with the same values
of @a data and @cleanup.
I think the wording I chose is most easily understood by a quick glance at the
docs and/or by people for whom English is not their first language, but I'm
open to better suggestions.
Thanks for the patch. This: + * The most recently registered cleanup with the same values of @a p, + * @a data and @a cleanup will be removed seems slightly ambiguous, as if there is some global list of cleanups indexed by (p, data, cleanup); I'd suggest instead: * The most cleanup most recently registered with p having the same values of * @a data and @a cleanup will be removed it would also worth noting that apr_pool_cleanup_for_exec runs all cleanups registered with the global pool and all children thereof. Any chance you could clean these these two things up and resubmit the patch to dev@apr.apache.org? Sure, I'll do it this weekend. Thanks for the comments. Created attachment 16231 [details] Clarify docs for cleanup functions Incorporates suggestions in comment 6. Thanks for the patch. Committed for trunk and 1.2.x. http://svn.apache.org/viewcvs?rev=264750&view=rev |