On 12/15/05, Julian Foad <julianfoad@btopenworld.com> wrote:
> Erik Huelsmann wrote:
> > In a previous mail
> > (http://svn.haxx.se/dev/archive-2005-12/0487.shtml), I tried to start
> > a discussion about the ownership model we (should) use when stacking
> > resources which need a destructor, such as streams (and files). Greg
> > Hudson confirmed later in that thread that the current model is the
> > one I think is sane too:
>
> (By "current model" you mean "currently proposed", not "currently implemented".)
Yes, but Greg ended the mail I quoted with a conclusion we're
currently not far from this model.
> > "
> > * Streams which read from or write to an underlying object own that
> > object, i.e. closing the stream closes the underlying object, if
> > applicable.
> >
> > * The layer (function or data type) which created a stream is
> > responsible for closing it, except when the above rule applies.
> >
> > * Window handlers are thought of as an odd kind of stream, and passing
> > the final NULL window is considered closing the stream.
> > "
> >
> > The original problem was that a stream_from_aprfile does not close the
> > apr file when the stream is closed, but I want to create an interface
> > which returns a stream (which is [sometimes] stacked on an apr file -
> > but that's an implementation detail). The above model says it's ok to
> > do that and expect the apr file to be closed. So far so good.
>
> It seems to me that:
>
> * svn_stream_from_aprfile() is clearly meant to make a stream that reads
> from or appends to an already-open file, and therefore it shouldn't
> close it. (Greg's second point.)
Oh, I really interpreted Greg's point differently. I interpreted him to say:
"svn_stream_from_aprfile() creates a stream which is stacked upon an
apr file. If the stacked stream is then closed, so should the
underlying file be."
> * If you want a stream that owns a file, the stream constructor should
> take the path as a parameter and open the file, and closing the stream
> should close the file.
More importantly than my interpretation above of Greg's point is that
if I have a look at svn_stream_translated(), if I close the translated
stream, it should close the stream on which it's stacked.
> Write a stream constructor that does this ("svn_stream_from_file") and
> leave both this and _from_aprfile available as alternatives, as both are
> useful.
This works for files, but I was thinking of the general rule, thinking
about files, streams and - as Greg pointed out - window handlers in
our editor interface.
> * The decision about whether the bottom-most stream owns its file should
> be made by the code that creates that stream from the file, and
> subsequent
> functions that play with the stream need not and should not be able
> to change
> that decision.
Absolutely right, but that wasn't what I'm talking about :-) What I
*am* talking about, I have a hard time putting in to words atm. I'll
followup after a good nights sleep.
> Isn't that the right solution?
>
>
> By the way, could you comment on the doc string of svn_stream_from_aprfile()?
> It contradicts svn_stream_from_aprfile(), which doesn't own the file, which is
> the implementation method for this.
>
> > /** Set @a *out to a generic stream connected to stdout, allocated in
> > * @a pool. The stream and its underlying APR handle will be closed
> > * when @a pool is cleared or destroyed.
> > */
> > svn_error_t *svn_stream_for_stdout (svn_stream_t **out, apr_pool_t *pool);
>
> Is that doc string just wrong?
No, it isn't, because the stream will be destroyed on pool
destruction. At the same time the apr file destruction handler will be
run, destroying the handle. Calling svn_stream_close on this stream
won't close the apr handle though.
bye,
Erik.
Received on Fri Dec 16 00:17:17 2005