[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix memory leak like situation when parsing a svn_config_t object repeatedly

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-07-24 21:12:03 CEST

Bernd Rinn wrote:
> Greg Hudson wrote:
>>I've committed r15404 with the workaround. I'll leave it to someone
>>else to code up the better long-term fix of revving the enumerate APIs.
>
> OK, I'll try. Please let me know if anything is inappropriate or needs
> to be changed.

This looks good, basically. Just a few comments:

The doc strings of the new functions should say "@since New in 1.3."; the doc
strings of the old functions should be rewritten as "Similar to oldfunction(),
but ...". Look at the doc strings of some other APIs that have been revved to
see how it was done.

> The changes are split in two parts: the first one revvs the API, the
> second one makes use of the new revision in order to replace calls to
> deprecated functions. The first patch can be applied independently of
> the second one.

That's fine. In this case the changes are small enough that it would be OK as
a single patch, but there's no harm in keeping them separate until someone
wants to combine them.

>
> Log message for svn_config_enumerate_pool_1.3_api.patch:
>
> [[[
> Create revision 2 of the API of function svn_config_enumerate() and
> svn_config_enumerate_sections().

That doesn't indicate why. Perhaps just add a few words like "to get the pool
handling right."

>
> Patch by: Bernd Rinn <bernd@sdf.lonestar.org>
>
> * subversion/libsvn_subr/config.c
> (svn_config_enumerate2): New. Copy of svn_config_enumerate() with
> cfg->x_pool replaced by the pool provided.
> (svn_config_sections2): New. Copy of svn_config_enumerate_sections()
> with cfg->x_pool replaced by the pool provided.
> (svn_config_enumerate): Deprecated. Create subpool of cfg->x_pool and
> call svn_config_enumerate2().
> apr_hash_first().

What's that line?

> (svn_config_sections): Deprecated. Create subpool of cfg->x_pool and
> call svn_config_enumerate_sections2().
>
> * subversion/include/svn_config.h
> (svn_config_enumerate2): New.
> (svn_config_sections2): New.
> (svn_config_enumerate): Deprecated.
> apr_hash_first().

And again!

> (svn_config_sections): Deprecated.
>
> ]]]
>
> Log message for
> svn_config_enumerate_pool_1.3_replace_deprecated_functioncalls.patch:
>
> [[[
> Replace calls to deprecated revision 1 of functions
> svn_config_enumerate() and svn_config_sections() by calls to revision 2.

That's fine: the word "deprecated" is enough to indicate why the calls are
being replaced.

>
> Patch by: Bernd Rinn <bernd@sdf.lonestar.org>
>
> * subversion/libsvn_client/add.c
> (svn_client__get_auto_props): Replace call to svn_config_enumerate
> with call to svn_config_enumerate2().
>
> * subversion/libsvn_repos/authz.c
> (authz_parse_section):
> (authz_get_path_access):
> (authz_get_tree_access):
> (authz_validate_section):
> (svn_repos_authz_read): Replace calls to svn_config_enumerate with
> call to svn_config_enumerate2(), and calls to
> svn_config_enumerate_sections() with calls to
> svn_config_enumerate_sections2().
> ]]]

Put commas betwen multiple symbold that have the same comment, and indent the
continuation lines of a paragraph, e.g.:

* subversion/libsvn_repos/authz.c
   (authz_parse_section, authz_get_path_access, authz_get_tree_access,
    authz_validate_section, svn_repos_authz_read):
     Replace calls to svn_config_enumerate with call to svn_config_enumerate2(),
     and calls to svn_config_enumerate_sections() with calls to
     svn_config_enumerate_sections2().

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 24 21:14:04 2005

This is an archived mail posted to the Subversion Dev mailing list.