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

Re: [PATCH] Fix xgettext warnings and incomplete format strings due to macros

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 1 Jun 2014 01:53:24 +0000

> Index: subversion/libsvn_fs_fs/cached_data.c
> ===================================================================
> --- subversion/libsvn_fs_fs/cached_data.c (revision 1598897)
> +++ subversion/libsvn_fs_fs/cached_data.c (working copy)
> @@ -941,9 +941,10 @@ svn_fs_fs__check_rep(representation_t *rep,
> || entry->type < SVN_FS_FS__ITEM_TYPE_FILE_REP
> || entry->type > SVN_FS_FS__ITEM_TYPE_DIR_PROPS)
> return svn_error_createf(SVN_ERR_REPOS_CORRUPTED, NULL,
> - _("No representation found at offset %s "
> - "for item %" APR_UINT64_T_FMT
> - " in revision %ld"),
> + apr_psprintf(pool,
> + _("No representation found at offset %%s "
> + "for item %%%s in revision %%ld"),
> + APR_UINT64_T_FMT),
> apr_off_t_toa(pool, offset),
> rep->item_index, rep->revision);
>

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?

> Index: subversion/libsvn_fs_fs/index.c
> ===================================================================
> --- subversion/libsvn_fs_fs/index.c (revision 1598897)
> +++ subversion/libsvn_fs_fs/index.c (working copy)
> l2p_page_info_copy(l2p_page_info_baton_t *baton,
> const l2p_header_t *header,
> const l2p_page_table_entry_t *page_table,
> - const apr_size_t *page_table_index)
> + const apr_size_t *page_table_index,
> + apr_pool_t *pool)
> {
> /* revision offset within the index file */
> apr_size_t rel_revision = baton->revision - header->first_revision;
> @@ -932,9 +936,10 @@ l2p_page_info_copy(l2p_page_info_baton_t *baton,
> * (last_entry - first_entry);
> if (baton->item_index >= max_item_index)
> return svn_error_createf(SVN_ERR_FS_ITEM_INDEX_OVERFLOW , NULL,
> - _("Item index %" APR_UINT64_T_FMT
> - " exceeds l2p limit of %" APR_UINT64_T_FMT
> - " for revision %ld"),
> + apr_psprintf(pool,
> + _("Item index %%%s exceeds l2p limit "
> + "of %%%s for revision %%ld"),
> + APR_UINT64_T_FMT, APR_UINT64_T_FMT),
> baton->item_index, max_item_index,
> baton->revision);
>
> @@ -970,7 +975,7 @@ l2p_page_info_access_func(void **out,
> (const void *const *)&header->page_table_index);
>
> /* 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.)

Thanks,

Daniel
Received on 2014-06-01 03:54:22 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.