Stefan Sperling wrote on Wed, Sep 22, 2010 at 14:15:52 +0200:
> On Wed, Sep 22, 2010 at 12:59:49PM +0200, Daniel Shahaf wrote:
> > Philip Martin wrote on Wed, Sep 22, 2010 at 11:47:19 +0100:
> > > Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
> > > > minval is an apr_uint64_t, so shouldn't the format string use APR_UINT64_FMT?
> > >
> > > See this thread
> > >
> > > http://svn.haxx.se/dev/archive-2010-09/0295.shtml
> > >
> >
> > and subversion/libsvn_fs_fs/rep-cache.c:153
>
> Daniel, the number parsing functions don't have a pool needed for the
> apr_psprintf() call, so they cannot use this approach.
>
Ah, right.
Although... in fact the pool is only needed for a constant-size
allocation (the format codes are three characters or less), not for an
arbitrary-sized one, so presumably we could arrange it to work even
without a pool (by allocating a format string with placeholders of the
form "foo%...bar%..." off the stack; the error is always allocated in
a global pool).
Not saying it's a good idea, just that it could theoreticaly work :)
> I'd say let's just stick to %ld for now, i.e. revert r999785.
>
But isn't that not always the correct format code?
> What we really need to do here is to decide whether or not we want to
> use APR for this at all. See this and related posts:
> http://svn.haxx.se/dev/archive-2010-09/0306.shtml
> The APR interface causes format string problems and lacks support for
> unsigned types. The C99 functions don't have these problems.
>
> As Philip pointed out, the format string problem is fixable by
> checking sizeof(long) at runtime. And we could request support for unsigned
> types in the APR interface, or even send a patch there. But that still
> leaves us with the question of what to do with current and older APR versions.
>
> I suppose the least painful route is to use C89/C99 constructs and switch
> all our svn_cstring number parsing functions over to using C89/C99 APIs
> instead of using APR.
> If we find that using these particular C99 functions (strtoll and stroull)
> or types (long long, which has been present in some compilers before C99),
> breaks Subversion with any compiler or C runtime in use today, we can add
> our own implementation for compatibility (BSD-licenced code is available),
> or fall back to APR on those platforms.
>
Sounds good. (to use the C99 functions, with their better-than-APR's
support for unsigned types, and add a private implementation if it's
needed)
> Stefan
Received on 2010-09-22 15:45:15 CEST