On Mon, 28 Apr 2003, Eric Dorland wrote:
> * rbb@rkbloom.net (rbb@rkbloom.net) wrote:
> >
> > > > As to why your commits fail, the problem is your use of
> > > > apr_file_data_get/apr_file_data_set. I do not know whether your
> > > > expectations are wrong, or whether the APR implementation is wrong,
> > > > but it appears that the data gets associated with the pool used by the
> > > > file, rather than with the file itself. This causes problems when two
> > > > files use the same pool.
> > >
> > > Yes I figured that out last night myself :) I think it's APR that's in
> > > the wrong here, since you would except the data to be associated with
> > > the file handle, not it's memory pool. Are there any APR developers
> > > on the list who think I'm wrong?
> >
> > <rbb raises hand>
> >
> > APR has called this out from the get-go. The data is associated with
> > whatever pool is used to open the file. The user-data stuff is an aspect
> > of pools, not every APR type. If we tried to associate the data with the
> > specific file handle, we would need a hash-table in every APR type, as
> > well as one in the pool. You must name your key so that it won't
> > interfere with other keys in the pool. This used to be very clearly
> > documented, but I haven't touched that code in well over a year, so I
> > don't know if it still is or not.
>
> Well let me say that's really stupid :-P
>
> Yeah, the docs for the file functions really don't make this
> clear, and from the name of the function I think my assumption would
> be the one most people would make. How many data structures do this
> besides apr_file_t? Is this behavior desired, or would you rather have
> the data associated with the actual object?
They all do it. The userdata stuff is an aspect of pools, not of the APR
types themselves. This is the desired behavior. It makes no sense at all
for every APR type to implement the userdata functions themselves. The
pools implement userdata, and all other types get it for free. (However,
I am no longer active in APR, so the actvie developers can feel free to
change this). The APR type could try to do the association for you, but
that would be a waste of time for anybody who named their keys so that
they didn't collide this way. The best solution is to name your key so
based on file handle.
However, if you really want my opinion, the user_data stuff in APR was
never fully thought out. I added it because Ken Coar suggested it would
be cool, and it didn't take long to write. I told Greg Stein a long time
ago that I thought it was a mistake to keep it in, but he said he really
liked the feature (it was at a booth at ApacheCon London, years ago). I
will say it again now, the fact that subversion uses the APR user_data
code is mildly scary to me, because it almost certainly doesn't do what
you expect it to do.
SVN would be much better served to create the hash table itself rather
than relying on APR userdata, and APR would be much better served by just
removing the userdata alltogether. It is a couple of bytes per pool that
are wasted 99% of the time.
Ryan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 29 01:03:37 2003