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