subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andreas Stieger <andreas.stie...@gmx.de>
Subject Re: [PATCH] Fix xgettext warnings and incomplete format strings due to macros
Date Sun, 01 Jun 2014 20:22:15 GMT
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

Mime
View raw message