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.
I'd say let's just stick to %ld for now, i.e. revert r999785.
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.
Stefan
Received on 2010-09-22 14:16:57 CEST