[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r999785 - /subversion/trunk/subversion/libsvn_subr/svn_string.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Wed, 22 Sep 2010 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.

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:
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.

Received on 2010-09-22 14:16:57 CEST

This is an archived mail posted to the Subversion Dev mailing list.