Hi,

Scanning the translations template file fails due to APR_UINT64_T_FMT
being used as is in format strings. This is from r1575628 (fsfs l2p
index), also from log-addressing branch, and r1595579 (sync of same to
FSX). While this works at execution time (modulo actually translating),
xgettext cannot handle that:

> make locale-gnu-pot
> Building subversion.pot...
> ../libsvn_fs_fs/index.c:597: warning: Although being used in a format string 
> position, the msgid is not a valid C format string. Reason: The string ends 
> in the middle of a directive.
> ../libsvn_fs_fs/index.c:657: warning: Although being used in a format string 
> position, the msgid is not a valid C format string. Reason: The string ends 
> in the middle of a directive.
> ../libsvn_fs_fs/cached_data.c:944: warning: Although being used in a format 
> string position, the msgid is not a valid C format string. Reason: The string 
> ends in the middle of a directive.
etc.

Please review the attached patch. It basically wraps the
APR_UINT64_T_FMT into a call to apr_psprintf. The introduction of a pool
parameter to some private functions seems necessary.

[[[
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 printing the
   format string macro separately

* 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 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 printing
   the format string macro separately
  (l2p_header_copy): same, and grow a pool parameter to support it
  (l2p_header_access_func,get_l2p_page_info): update calls to
   l2p_header_copy to pass a pool
]]]

With kind regards,
Andreas Stieger
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);
 
Index: subversion/libsvn_fs_fs/index.c
===================================================================
--- subversion/libsvn_fs_fs/index.c	(revision 1598897)
+++ subversion/libsvn_fs_fs/index.c	(working copy)
@@ -594,8 +594,10 @@ svn_fs_fs__l2p_index_create(svn_fs_t *fs,
    * The current implementation is limited to 2G entries per page. */
   if (ffd->l2p_page_size > APR_INT32_MAX)
     return svn_error_createf(SVN_ERR_FS_ITEM_INDEX_OVERFLOW , NULL,
-                            _("L2P index page size  %" APR_UINT64_T_FMT
-                              " exceeds current limit of 2G entries"),
+                            apr_psprintf(pool,
+                              _("L2P index page size  %%%s" 
+                                " exceeds current limit of 2G entries"),
+                            APR_UINT64_T_FMT),
                             ffd->l2p_page_size);
 
   /* start at the beginning of the source file */
@@ -654,9 +656,10 @@ svn_fs_fs__l2p_index_create(svn_fs_t *fs,
           /* store the mapping in our array */
           if (proto_entry.item_index > APR_INT32_MAX)
             return svn_error_createf(SVN_ERR_FS_ITEM_INDEX_OVERFLOW , NULL,
-                                    _("Item index %" APR_UINT64_T_FMT
-                                      " too large in l2p proto index for"
-                                      " revision %ld"),
+                                    apr_psprintf(pool,
+                                      _("Item index %%%s too large "
+                                        "in l2p proto index for revision %%ld"),
+                                    APR_UINT64_T_FMT),
                                     proto_entry.item_index,
                                     revision + page_counts->nelts);
 
@@ -900,7 +903,8 @@ static svn_error_t *
 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);
 }
 
 /* Get the page info requested in *BATON from FS and set the output fields
@@ -1002,7 +1007,7 @@ get_l2p_page_info(l2p_page_info_baton_t *baton,
   /* read from disk, cache and copy the result */
   SVN_ERR(get_l2p_header_body(&result, rev_file, fs, baton->revision, pool));
   SVN_ERR(l2p_page_info_copy(baton, result, result->page_table,
-                             result->page_table_index));
+                             result->page_table_index, pool));
 
   return SVN_NO_ERROR;
 }
@@ -1241,13 +1246,16 @@ typedef struct l2p_entry_baton_t
 static svn_error_t *
 l2p_page_get_entry(l2p_entry_baton_t *baton,
                    const l2p_page_t *page,
-                   const apr_uint64_t *offsets)
+                   const apr_uint64_t *offsets,
+                   apr_pool_t *pool)
 {
   /* overflow check */
   if (page->entry_count <= baton->page_offset)
     return svn_error_createf(SVN_ERR_FS_ITEM_INDEX_OVERFLOW , NULL,
-                             _("Item index %" APR_UINT64_T_FMT
-                               " too large in revision %ld"),
+                             apr_psprintf(pool,
+                               _("Item index %%%s"
+                                 " too large in revision %%ld"),
+                             APR_UINT64_T_FMT),
                              baton->item_index, baton->revision);
 
   /* return the result */
@@ -1273,7 +1281,7 @@ l2p_entry_access_func(void **out,
     = svn_temp_deserializer__ptr(page, (const void *const *)&page->offsets);
 
   /* return the requested data */
-  return l2p_page_get_entry(baton, page, offsets);
+  return l2p_page_get_entry(baton, page, offsets, result_pool);
 }
 
 /* Using the log-to-phys indexes in FS, find the absolute offset in the
@@ -1338,7 +1346,7 @@ l2p_index_lookup(apr_off_t *offset,
 
       /* cache the page and extract the result we need */
       SVN_ERR(svn_cache__set(ffd->l2p_page_cache, &key, page, pool));
-      SVN_ERR(l2p_page_get_entry(&page_baton, page, page->offsets));
+      SVN_ERR(l2p_page_get_entry(&page_baton, page, page->offsets, pool));
 
       /* prefetch pages from following and preceding revisions */
       pages = apr_array_make(pool, 16, sizeof(l2p_page_table_entry_t));
Index: subversion/libsvn_fs_x/cached_data.c
===================================================================
--- subversion/libsvn_fs_x/cached_data.c	(revision 1598897)
+++ subversion/libsvn_fs_x/cached_data.c	(working copy)
@@ -749,9 +749,10 @@ svn_fs_x__check_rep(representation_t *rep,
           && entry->type != SVN_FS_X__ITEM_TYPE_DIR_PROPS
           && entry->type != SVN_FS_X__ITEM_TYPE_REPS_CONT))
     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->id.number, revision);
 
Index: subversion/libsvn_fs_x/index.c
===================================================================
--- subversion/libsvn_fs_x/index.c	(revision 1598897)
+++ subversion/libsvn_fs_x/index.c	(working copy)
@@ -719,8 +719,10 @@ svn_fs_x__l2p_index_create(svn_fs_t *fs,
    * The current implementation is limited to 2G entries per page. */
   if (ffd->l2p_page_size > APR_INT32_MAX)
     return svn_error_createf(SVN_ERR_FS_ITEM_INDEX_OVERFLOW , NULL,
-                            _("L2P index page size  %" APR_UINT64_T_FMT
-                              " exceeds current limit of 2G entries"),
+                            apr_psprintf(pool,
+                              _("L2P index page size  %%%s" 
+                                " exceeds current limit of 2G entries"),
+                            APR_UINT64_T_FMT),
                             ffd->l2p_page_size);
 
   /* start at the beginning of the source file */
@@ -781,9 +783,10 @@ svn_fs_x__l2p_index_create(svn_fs_t *fs,
 
           if (proto_entry.item_index > APR_INT32_MAX)
             return svn_error_createf(SVN_ERR_FS_ITEM_INDEX_OVERFLOW , NULL,
-                                    _("Item index %" APR_UINT64_T_FMT
-                                      " too large in l2p proto index for"
-                                      " revision %ld"),
+                                    apr_psprintf(pool,
+                                      _("Item index %%%s too large "
+                                        "in l2p proto index for revision %%ld"),
+                                    APR_UINT64_T_FMT),
                                     proto_entry.item_index,
                                     revision + page_counts->nelts);
 
@@ -907,7 +910,8 @@ static svn_error_t *
 l2p_header_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;
@@ -939,9 +943,10 @@ l2p_header_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);
 
@@ -977,7 +982,7 @@ l2p_header_access_func(void **out,
                            (const void *const *)&header->page_table_index);
 
   /* copy the info */
-  return l2p_header_copy(baton, header, page_table, page_table_index);
+  return l2p_header_copy(baton, header, page_table, page_table_index, result_pool);
 }
 
 /* Read COUNT run-length-encoded (see rle_array) uint64 from STREAM and
@@ -1139,7 +1144,7 @@ get_l2p_page_info(l2p_page_info_baton_t *baton,
   /* read from disk, cache and copy the result */
   SVN_ERR(get_l2p_header_body(&result, stream, fs, baton->revision, pool));
   SVN_ERR(l2p_header_copy(baton, result, result->page_table,
-                          result->page_table_index));
+                          result->page_table_index, pool));
 
   return SVN_NO_ERROR;
 }

Reply via email to