You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Tobias Ringström <to...@ringstrom.mine.nu> on 2004/01/24 00:51:11 UTC

Yet another look at the apr_off_t problem

The only real problem is that the size of apr_off_t is variable 
depending on compiler options (because it is an off_t typedef).

Greg Hudson proposed to make apr_off_t 64 bit, but the drawback of doing 
so is that it cannot be done for apr 0.9 because the ABI would change. 
Another appealing solution is to simply modify apr to make the size of 
apr_off_t fixed to the size of off_t when apr is compiled. What I'm 
saying is that apr should have an autoconf test to find out the size of 
off_t, and use that size when creating the apr_off_t typedef.

Right now it is

	typedef off_t apr_off_t;

but after such a change it should be something like

	#if APR_HAS_32BIT_OFF_T
	typedef apr_int32_t apr_off_t;
	...

if autoconf finds out that off_t is 32 bit at the time when apr is 
configured and compiled.

The advantage of doing this is that we can release Subversion right now 
without changes.  With current apr, the perl bindings will be broken, 
but when compiled with a fixed apr, they will just work.

The drawback is of course that someone need to change apr, and I don't 
know who easy that is, or how fast it can be done.

Does this sound like a plan or what?

/Tobias, sleepless


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Yet another look at the apr_off_t problem

Posted by Ben Reser <be...@reser.org>.
On Sat, Jan 24, 2004 at 03:32:36PM +0000, Julian Foad wrote:
> I understand the explanation that has been given for why the current 
> "apr_off_t" is unsuitable for use in a library API, but I am not completely 
> familiar with library API/ABI design issues.  The use of "apr_uint64_t" to 
> hold a line number seems to me very ugly.  A line number is not logically a 
> 64-bit type, it is an unbounded type.  Interfaces within an application are 
> normally best written with plain "int".  If "int" is not big enough for 
> someone's line numbers, then they almost certainly need a bigger computer 
> (one which is natively 64-bit or 128-bit).
> 
> Is there an ABI-related reason not to use plain "int" in library APIs?  If 
> so, what about our several other APIs that already use plain "int"?

Nope a line number is not an unbound type.  It is limited to the maximum
number of characters in a file (as long as you start numbering with 0).
The maximum sized apr_off_t and the maximum file size we support is an
int64, which is why we are using an apr_int64_t for this type here (not
apr_uint64_t which was used in earlier versions and bliss convinced me
it was unnecessary and bad).

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Yet another look at the apr_off_t problem

Posted by Travis P <sv...@castle.fastmail.fm>.
On Jan 24, 2004, at 9:32 AM, Julian Foad wrote:

>  If "int" is not big enough for someone's line numbers, then they  
> almost certainly need a bigger computer (one which is natively 64-bit  
> or 128-bit).

I do not understand that statement.  The native capability of the  
machine will probably not have any effect on the size of the int type  
(I know it won't for xlc or gcc and give references below).  If you  
write APIs assuming that ints will increase in size based on native  
machine register width, it likely won't work as you expect.  If line  
numbers are represented by ints, then 2^31 lines is most likely the  
maximum that will ever work, independent of machine register sizes.

Some references for those interested:

* On the Power/PowerPC front, which has been 64-bit for a relatively  
long time:

   AIX 5L Porting Guide, Chapter 3. Issues regarding 32-bit and 64-bit
    
<http://publib-b.boulder.ibm.com/Redbooks.nsf/ 
9445fa5b416f6e32852569ae006bb65f/d34977afad1e52aa86256a9300680bcc? 
OpenDocument&Highlight=0,64-bit>

   I'm also rather partial to the older "AIX 64-bit Performance in  
Focus" which can no longer be found by searching, but here's a direct  
link: http://www.redbooks.ibm.com/redbooks/pdfs/sg245103.pdf

* On the AMD/gcc front,

    
http://gcc.gnu.org/onlinedocs/gcc-3.3.2/gcc/i386-and-x86-64- 
Options.html#i386%20and%20x86-64%20Options
     -m32
     -m64
     Generate code for a 32-bit or 64-bit environment. The 32-bit  
environment sets int, long and pointer to 32 bits and generates code  
that runs on any i386 system. The 64-bit environment sets int to 32  
bits and long and pointer to 64 bits and generates code for AMD's  
x86-64 architecture.


FWIW, explicitly choosing 64-bit integer values for line numbers  
appears to be a smart move to me.

-Travis


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Yet another look at the apr_off_t problem

Posted by Julian Foad <ju...@btopenworld.com>.
Ben Reser wrote:
> On Fri, Jan 23, 2004 at 05:44:21PM -0800, Ben Reser wrote:
> 
>>Make the blame fix because the line_no parameter doesn't make any sense
>>being an apr_off_t type.  We change that to apr_uint64_t.  We put that
>>in 0.37.0.  This fixes the blame problem.
> 
> The blame patch is now attached to Issue #1705 as attachment 291.

I understand the explanation that has been given for why the current "apr_off_t" is unsuitable for use in a library API, but I am not completely familiar with library API/ABI design issues.  The use of "apr_uint64_t" to hold a line number seems to me very ugly.  A line number is not logically a 64-bit type, it is an unbounded type.  Interfaces within an application are normally best written with plain "int".  If "int" is not big enough for someone's line numbers, then they almost certainly need a bigger computer (one which is natively 64-bit or 128-bit).

Is there an ABI-related reason not to use plain "int" in library APIs?  If so, what about our several other APIs that already use plain "int"?

- Julian


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Yet another look at the apr_off_t problem

Posted by Ben Reser <be...@reser.org>.
On Fri, Jan 23, 2004 at 05:44:21PM -0800, Ben Reser wrote:
> Make the blame fix because the line_no parameter doesn't make any sense
> being an apr_off_t type.  We change that to apr_uint64_t.  We put that
> in 0.37.0.  This fixes the blame problem.

The blame patch is now attached to Issue #1705 as attachment 291.

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Yet another look at the apr_off_t problem

Posted by Ben Reser <be...@reser.org>.
On Sat, Jan 24, 2004 at 01:51:11AM +0100, Tobias Ringström wrote:
> The only real problem is that the size of apr_off_t is variable 
> depending on compiler options (because it is an off_t typedef).
> 
> Greg Hudson proposed to make apr_off_t 64 bit, but the drawback of doing 
> so is that it cannot be done for apr 0.9 because the ABI would change. 
> Another appealing solution is to simply modify apr to make the size of 
> apr_off_t fixed to the size of off_t when apr is compiled. What I'm 
> saying is that apr should have an autoconf test to find out the size of 
> off_t, and use that size when creating the apr_off_t typedef.
> 
> Right now it is
> 
> 	typedef off_t apr_off_t;
> 
> but after such a change it should be something like
> 
> 	#if APR_HAS_32BIT_OFF_T
> 	typedef apr_int32_t apr_off_t;
> 	...
> 
> if autoconf finds out that off_t is 32 bit at the time when apr is 
> configured and compiled.

To clarify for people that didn't understand what he means here... Let
me give a simple explanation of the problem.

apr_off_t is typedefed from off_t.  off_t on some platforms can be a
32-bit or 64-bit integer depending on compile time flags.

If the compile time flags for when APR is built and a program using APR
is built differ then there can be an ABI incopatability.

The change proposed here would shift the determination of the size of
apr_off_t from when any code using it is built to when APR is built.     

If the compile time flags for APR make the platform provide it a 32-bit
off_t then APR would typedef apr_off_t as:
typedef apr_int32_t apr_off_t

If the compile time flags for APR makes the platform provide it a 64-bit
off_t then APR would typedef apr_off_t as:
typedef apr_int64_t apr_off_t

Even if an application built with an off_t of a different size than what
APR was built with apr_off_t would always be the same size as APR thinks
it is.  The ABI is preserved.

> The advantage of doing this is that we can release Subversion right now 
> without changes.  With current apr, the perl bindings will be broken, 
> but when compiled with a fixed apr, they will just work.
> 
> The drawback is of course that someone need to change apr, and I don't 
> know who easy that is, or how fast it can be done.
> 
> Does this sound like a plan or what?

This would be an acceptable solution.  Iff APR is willing to make such a
change.

So how about the following:

Make the blame fix because the line_no parameter doesn't make any sense
being an apr_off_t type.  We change that to apr_uint64_t.  We put that
in 0.37.0.  This fixes the blame problem.

We don't do anything about any of the other issues in 0.37.0.  We try
and work with the APR people to fix the issue in APR during the soak
time.  If it becomes clear that such a solution isn't acceptable to APR
then we'll have to revist the issue, decide what our options are from
there, and what if any impact those options have on the 1.0 release.

Sound fair?

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Yet another look at the apr_off_t problem

Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> On Fri, 2004-01-23 at 19:51, Tobias Ringström wrote:
> > What I'm 
> > saying is that apr should have an autoconf test to find out the size of 
> > off_t, and use that size when creating the apr_off_t typedef.
> 
> I am okay with that.  Karl will like the idea because it means
> Subversion doesn't have to change.

You betcha :-).

And I'm willing to contribute to the "work with APR" part (as an APR
developer, albeit a rather inactive one lately) during Subversion's
soak period.

> Ben still wants to change blame to use a fixed-size type (as a practical
> expedient) and to make svn_filesize_t signed. I will vote for those
> changes, but I won't champion them.

I'll take a look at #1705 (attachment 291) and #1713 right now.

Josander isn't rolling until about 6pm tomorrow night, his time :-).


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Yet another look at the apr_off_t problem

Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2004-01-23 at 19:51, Tobias Ringström wrote:
> What I'm 
> saying is that apr should have an autoconf test to find out the size of 
> off_t, and use that size when creating the apr_off_t typedef.

I am okay with that.  Karl will like the idea because it means
Subversion doesn't have to change.

Ben still wants to change blame to use a fixed-size type (as a practical
expedient) and to make svn_filesize_t signed. I will vote for those
changes, but I won't champion them.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org