You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> on 2007/12/27 19:01:33 UTC
[PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c (Was: svn commit: r28368 - trunk/subversion/libsvn_repos)
2007-12-15 16:41:01 Dongsheng Song napisał(a):
> Yes, the current pot is bad:
>
> #: ../libsvn_repos/reporter.c:168
> msgid "Invalid length (%"
> msgstr ""
>
> If apply the following patch, it's good:
>
> #: ../libsvn_repos/reporter.c:168
> #, c-format
> msgid "Invalid length (%%%s) when about to read a string"
> msgstr ""
>
> ===================================================================
> --- subversion/libsvn_repos/reporter.c (revision 28499)
> +++ subversion/libsvn_repos/reporter.c (working copy)
> @@ -155,6 +155,7 @@
> {
> apr_uint64_t len;
> char *buf;
> + char *fmt;
>
> SVN_ERR(read_number(&len, temp, pool));
>
> @@ -164,9 +165,10 @@
> string, anyone?) but let's be future-proof anyway. */
> if (len + 1 < len)
> {
> + fmt = apr_psprintf(pool, _("Invalid length (%%%s) when about to read
> "
> + "a string"), APR_UINT64_T_FMT);
> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
> - _("Invalid length (%" APR_UINT64_T_FMT ") "
> - "when about to read a string"), len);
> + fmt, len);
> }
>
> buf = apr_palloc(pool, len + 1);
>
I've reviewed this patch and I'm confirming that it's correct, however
'char *fmt;' isn't necessary, so I'm attaching the updated patch.
Could any full committer review it?
[[[
* subversion/libsvn_repos/reporter.c
(read_string): Use apr_psprintf to create translatable string.
Patch by: cauchy
arfrever
Suggested by: dionisos
]]]
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2007-12-27 23:59:05 David Glasser napisał(a):
> On Dec 27, 2007 2:27 PM, Arfrever Frehtes Taifersar Arahesis
> <ar...@gmail.com> wrote:
> > 2007-12-27 23:13:39 Karl Fogel napisa�(a):
> > > Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> > > > 2007-12-15 16:41:01 Dongsheng Song napisa�(a):
> > > >> ===================================================================
> > > >> --- subversion/libsvn_repos/reporter.c (revision 28499)
> > > >> +++ subversion/libsvn_repos/reporter.c (working copy)
> > > >> @@ -155,6 +155,7 @@
> > > >> {
> > > >> apr_uint64_t len;
> > > >> char *buf;
> > > >> + char *fmt;
> > > >>
> > > >> SVN_ERR(read_number(&len, temp, pool));
> > > >>
> > > >> @@ -164,9 +165,10 @@
> > > >> string, anyone?) but let's be future-proof anyway. */
> > > >> if (len + 1 < len)
> > > >> {
> > > >> + fmt = apr_psprintf(pool, _("Invalid length (%%%s) when about to read
> > > >> "
> > > >> + "a string"), APR_UINT64_T_FMT);
> > > >> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
> > > >> - _("Invalid length (%" APR_UINT64_T_FMT ") "
> > > >> - "when about to read a string"), len);
> > > >> + fmt, len);
> > > >> }
> > > >>
> > > >> buf = apr_palloc(pool, len + 1);
> > > >>
> >
> > This 'fmt' isn't necessary.
> > What do you think about this version?:
> >
> > 2007-12-27 20:01:10 Arfrever Frehtes Taifersar Arahesis napisa�(a):
> > > Index: subversion/libsvn_repos/reporter.c
> > > ===================================================================
> > > --- subversion/libsvn_repos/reporter.c (wersja 28655)
> > > +++ subversion/libsvn_repos/reporter.c (kopia robocza)
> > > @@ -165,8 +165,11 @@
> > > if (len + 1 < len)
> > > {
> > > return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
> > > - _("Invalid length (%" APR_UINT64_T_FMT ") "
> > > - "when about to read a string"), len);
> > > + apr_psprintf(pool,
> > > + _("Invalid length (%%%s) when "
> > > + "about to read a string"),
> > > + APR_UINT64_T_FMT),
> > > + len);
> > > }
> > >
> > > buf = apr_palloc(pool, len + 1);
> > >
> >
>
> How about a comment explaining why we're jumping through these hoops?
A comment in the source code or in svn:log?
And what do you think about this comment?:
"xgettext doesn't expand preprocessor definitions, so we must use apr_psprintf to create intermediate string with appropriate format specifier."
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2007-12-28 09:49:24 Karl Fogel napisał(a):
> Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> > This 'fmt' isn't necessary.
> > What do you think about this version?:
>
> I like the new version fine, thanks. To answer your other mail, the
> explanatory comment should go in the code, not the log message.
>
> -Karl
>
> > 2007-12-27 20:01:10 Arfrever Frehtes Taifersar Arahesis napisał(a):
> >> Index: subversion/libsvn_repos/reporter.c
> >> ===================================================================
> >> --- subversion/libsvn_repos/reporter.c (wersja 28655)
> >> +++ subversion/libsvn_repos/reporter.c (kopia robocza)
> >> @@ -165,8 +165,11 @@
> >> if (len + 1 < len)
> >> {
> >> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
> >> - _("Invalid length (%" APR_UINT64_T_FMT ") "
> >> - "when about to read a string"), len);
> >> + apr_psprintf(pool,
> >> + _("Invalid length (%%%s) when "
> >> + "about to read a string"),
> >> + APR_UINT64_T_FMT),
> >> + len);
> >> }
> >>
> >> buf = apr_palloc(pool, len + 1);
> >>
Committed in r28692.
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c
Posted by Karl Fogel <kf...@red-bean.com>.
Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> This 'fmt' isn't necessary.
> What do you think about this version?:
I like the new version fine, thanks. To answer your other mail, the
explanatory comment should go in the code, not the log message.
-Karl
> 2007-12-27 20:01:10 Arfrever Frehtes Taifersar Arahesis napisał(a):
>> Index: subversion/libsvn_repos/reporter.c
>> ===================================================================
>> --- subversion/libsvn_repos/reporter.c (wersja 28655)
>> +++ subversion/libsvn_repos/reporter.c (kopia robocza)
>> @@ -165,8 +165,11 @@
>> if (len + 1 < len)
>> {
>> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
>> - _("Invalid length (%" APR_UINT64_T_FMT ") "
>> - "when about to read a string"), len);
>> + apr_psprintf(pool,
>> + _("Invalid length (%%%s) when "
>> + "about to read a string"),
>> + APR_UINT64_T_FMT),
>> + len);
>> }
>>
>> buf = apr_palloc(pool, len + 1);
>>
>
>
> --
> Arfrever Frehtes Taifersar Arahesis
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2007-12-27 23:13:39 Karl Fogel napisał(a):
> Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> > 2007-12-15 16:41:01 Dongsheng Song napisał(a):
> >> ===================================================================
> >> --- subversion/libsvn_repos/reporter.c (revision 28499)
> >> +++ subversion/libsvn_repos/reporter.c (working copy)
> >> @@ -155,6 +155,7 @@
> >> {
> >> apr_uint64_t len;
> >> char *buf;
> >> + char *fmt;
> >>
> >> SVN_ERR(read_number(&len, temp, pool));
> >>
> >> @@ -164,9 +165,10 @@
> >> string, anyone?) but let's be future-proof anyway. */
> >> if (len + 1 < len)
> >> {
> >> + fmt = apr_psprintf(pool, _("Invalid length (%%%s) when about to read
> >> "
> >> + "a string"), APR_UINT64_T_FMT);
> >> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
> >> - _("Invalid length (%" APR_UINT64_T_FMT ") "
> >> - "when about to read a string"), len);
> >> + fmt, len);
> >> }
> >>
> >> buf = apr_palloc(pool, len + 1);
> >>
This 'fmt' isn't necessary.
What do you think about this version?:
2007-12-27 20:01:10 Arfrever Frehtes Taifersar Arahesis napisał(a):
> Index: subversion/libsvn_repos/reporter.c
> ===================================================================
> --- subversion/libsvn_repos/reporter.c (wersja 28655)
> +++ subversion/libsvn_repos/reporter.c (kopia robocza)
> @@ -165,8 +165,11 @@
> if (len + 1 < len)
> {
> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
> - _("Invalid length (%" APR_UINT64_T_FMT ") "
> - "when about to read a string"), len);
> + apr_psprintf(pool,
> + _("Invalid length (%%%s) when "
> + "about to read a string"),
> + APR_UINT64_T_FMT),
> + len);
> }
>
> buf = apr_palloc(pool, len + 1);
>
--
Arfrever Frehtes Taifersar Arahesis
Re: [PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c
Posted by Karl Fogel <kf...@red-bean.com>.
Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> 2007-12-15 16:41:01 Dongsheng Song napisał(a):
>> Yes, the current pot is bad:
>>
>> #: ../libsvn_repos/reporter.c:168
>> msgid "Invalid length (%"
>> msgstr ""
>>
>> If apply the following patch, it's good:
>>
>> #: ../libsvn_repos/reporter.c:168
>> #, c-format
>> msgid "Invalid length (%%%s) when about to read a string"
>> msgstr ""
You can declare 'fmt' inside the 'if' block. Other than that, +1 to
commit (I assume you've tested), and thanks for the fix!
-Karl
>> ===================================================================
>> --- subversion/libsvn_repos/reporter.c (revision 28499)
>> +++ subversion/libsvn_repos/reporter.c (working copy)
>> @@ -155,6 +155,7 @@
>> {
>> apr_uint64_t len;
>> char *buf;
>> + char *fmt;
>>
>> SVN_ERR(read_number(&len, temp, pool));
>>
>> @@ -164,9 +165,10 @@
>> string, anyone?) but let's be future-proof anyway. */
>> if (len + 1 < len)
>> {
>> + fmt = apr_psprintf(pool, _("Invalid length (%%%s) when about to read
>> "
>> + "a string"), APR_UINT64_T_FMT);
>> return svn_error_createf(SVN_ERR_REPOS_BAD_REVISION_REPORT, NULL,
>> - _("Invalid length (%" APR_UINT64_T_FMT ") "
>> - "when about to read a string"), len);
>> + fmt, len);
>> }
>>
>> buf = apr_palloc(pool, len + 1);
>>
>
> I've reviewed this patch and I'm confirming that it's correct, however
> 'char *fmt;' isn't necessary, so I'm attaching the updated patch.
>
> Could any full committer review it?
>
> [[[
> * subversion/libsvn_repos/reporter.c
> (read_string): Use apr_psprintf to create translatable string.
>
> Patch by: cauchy
> arfrever
>
> Suggested by: dionisos
> ]]]
>
>
> --
> Arfrever Frehtes Taifersar Arahesis
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org