Re: [PATCH] Fix wrong string in subversion/libsvn_repos/reporter.c

2007-12-27 23:59:05 David Glasser napisaƂ(a):
> On Dec 27, 2007 2:27 PM, Arfrever Frehtes Taifersar Arahesis
> <arfrever.fta@gmail.com> wrote:
> > 2007-12-27 23:13:39 Karl Fogel napisał(a):
> > > Arfrever Frehtes Taifersar Arahesis <arfrever.fta@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