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

Re: svn commit: r995475 - /subversion/trunk/subversion/libsvn_repos/load.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 10 Sep 2010 16:56:07 +0200

On Fri, Sep 10, 2010 at 04:02:52PM +0200, Branko Čibej wrote:
> On 10.09.2010 15:26, Stefan Sperling wrote:
> > I know we're using C89, but maybe it's time to move on and upgrade to C99
> > where the benefits are desirable? When Subversion was started, C89 was about
> > a decade old, and C99 is just as old now...
>
> Microsoft's C compiler, to name only one, still does not provide most of
> the C99 features. I don't know about standard library support.
>
> However, I don't see where you gain with using strtol(). First of all,
> it was already in C89 and has effectively the same interface as the APR
> conversions. C99 added strtoll() but it has the same interface. What's
> the benefit?

Hmmm.. looking closer, you're right, the interface is the same.
It's just that the programmer (me) has to use it properly.
The strtol(3) man page has an interesting hint which is missing from
the apr_strtoi64() documentation. It wasn't clear to me how to make
sure that the entire string was a valid number, but this explains it:

     If endptr is non-null, strtol() stores the address of the first invalid
     character in *endptr. If there were no digits at all, however, strtol()
     stores the original value of nptr in *endptr. (Thus, if *nptr is not
     `\0' but **endptr is `\0' on return, the entire string was valid.)

I found this snippet in fs_fs.c (which has since been changed, I'm showing
an old version), where the caller is happy with a string like "42\n",
but potentially accepts "42foo":

  /* note that apr_atoi64() will stop reading as soon as it encounters
     the final newline. */
  if (changes_offset)
    *changes_offset = rev_offset + apr_atoi64(&buf[i]);

So that caller will need to be adjusted.

But yes, checking whether *endptr == '\0' should do the trick.
So I guess we can keep using apr_atoi64(). Thanks for the cluestick :)

Stefan
Received on 2010-09-10 16:56:51 CEST

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