Hi,
thank you for the review.
On 01/06/14 02:53, Daniel Shahaf wrote:
> Don't we prefer doing:
>
> svn_error_createf(SVN_ERR_BASE, NULL, _("%s: number %ld"),
> apr_psprintf(pool, "%" APR_UINT64_T_FMT, (apr_uint64_t)1),
> 1L)
>
> since it allows for compile-time type checking of varargs against the
> format string?
Yes, adjusted accordingly, and fixed another instance of same in
l2p_page_get_offset which was previously attempted by someone else.
>> /* copy the info */
>> - return l2p_page_info_copy(baton, header, page_table, page_table_index);
>> + return l2p_page_info_copy(baton, header, page_table, page_table_index, result_pool);
>
> That should be scratch_pool, since you only use it to allocate an error
> message. (The svn_error_t->message member is constructed in the error's
> pool, which is the child of a global pool, not related to any of the
> pools in this scope.)
As discussed on IRC, a scratch_pool in not available. Changed to
local_pool where it is created in the function. Again review for the
updated patch would be appreciated, especially on the pool usage in the
context.
[[[
Fix xgettext warnings and incomplete format strings due to macros
* subversion/libsvn_fs_fs/cached_data.c
(svn_fs_fs__check_rep): make xgettext friendly by removing the
macro from the format string
* subversion/libsvn_fs_x/cached_data.c
(svn_fs_x__check_rep): same
* subversion/libsvn_fs_fs/index.c
(svn_fs_fs__l2p_index_create): same x2
(l2p_page_info_copy,l2p_page_get_entry): same, and grow
scratch_pool parameters to support it
(l2p_page_info_access_func,get_l2p_page_info): update calls to
l2p_page_info_copy to pass a pool
(l2p_entry_access_func,l2p_index_lookup): update calls to
l2p_page_get_entry to pass a pool
* subversion/libsvn_fs_x/index.c
(svn_fs_x__l2p_index_create): make xgettext friendly by removing
the macro from the format string
(l2p_header_copy): same, and grow scratch_pool parameters to
support it
(l2p_header_access_func,get_l2p_page_info): update calls to
l2p_header_copy to pass a pool
(l2p_page_get_offset): same pattern to allow compile-time
checking of arguments against format string
]]]
Andreas
Received on 2014-06-01 22:22:56 CEST