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

A new look at the apr_off_t problem

From: Greg Hudson <ghudson_at_MIT.EDU>
Date: 2004-01-23 23:16:14 CET

Tobias has discovered a twist to the apr_off_t problem which requires
us to take a step back.

As I've detailed at length, using apr_off_t in our APIs is bad because
(in decreasing order of importance):

  * It interacts badly with perl right now.
  * It interacts badly with a possible future APR change
    (see http://marc.theaimsgroup.com/?l=apr-dev&m=107481332607151&w=20)
  * It interacts badly with callers or callees which might be built
    with _FILE_OFFSET_BITS64.

Unfortunately, eliminating it from our API is hard or worrisome
because:

  * We use it in lots of places in the diff interface, and fixing that
    happens to dovetail into a secondary problem, that the diff
    interface doesn't use pools in a lot of places where it probably
    should.

  * Our svn_io APR wrappers use apr_finfo_t, which contains an
    apr_off_t. We can't reasonably duplicate the apr_finfo_t
    structure to work around this problem.

  * svn 0.37 comes out tomorrow, and some of us were really hoping
    that it would be equal to svn 1.0 in all respects for the version
    number. Putting in a big change now isn't great.

So, we need to think about targeted solutions. There are three areas
of the API in play here:

  1. The blame callback uses an apr_off_t for the line number. It is
     easy to change this to use an apr_uint64_t instead. (The current
     #1710 patch does this.)

  2. The diff library uses apr_off_t all over the place. We can fix
     that by introducing svn_offset_t, and also introducing pool
     parameters all over the place. This is a good change, but it's
     also a big change, and not necessarily appropriate for 0.37.
     (The current #1710 does this.)

  3. The svn_io wrappers use apr_off_t and apr_finfo_t. The best we
     could do here is to declare those wrappers to be private for now,
     which would be a giant rename. (The current #1710 patch uses
     svn_offset_t in places of apr_off_t, but does not address
     apr_finfo_t.)

Although (2) and (3) may bite us if we don't fix them (particularly if
APR 1.0 changes the meaning of apr_off_t), the most important thing to
fix is (1), because without that fix, we can't have working perl
wrappers for blame. (Without (2), we can't have working perl wrappers
for libsvn_diff, but I don't know if such a thing is really important,
and Joe Orton thinks it may be possible to fix perl in the long run.)
Certainly, we don't care about perl wrappers for (3).

So, I think the thing to do right now is create a patch for (1) (it
will be something like a one-line change) and merge it in tonight for
0.37. On a slightly more leisurely schedule, we can work out a big
patch for (2) and (3). Then we can argue about whether it's
appropriate to roll an 0.38 in order to get that patch in.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 23 23:16:50 2004

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