You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <rh...@sharpsvn.net> on 2009/03/24 21:35:49 UTC
RE: svn commit: r36747 - trunk/subversion/libsvn_subr
> -----Original Message-----
> From: Ivan Zhakov [mailto:chemodax@gmail.com]
> Sent: Monday, March 23, 2009 8:25 PM
> To: svn@subversion.tigris.org
> Subject: svn commit: r36747 - trunk/subversion/libsvn_subr
>
> Author: zhakov
> Date: Mon Mar 23 12:24:54 2009
> New Revision: 36747
>
> Log:
> * subversion/libsvn_subr/subst.c:
> (svn_subst_translate_cstring2): Close the destination stream to flush
> unwritten data.
>
> Modified:
> trunk/subversion/libsvn_subr/subst.c
>
> Modified: trunk/subversion/libsvn_subr/subst.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/subst.c?p
> athrev=36747&r1=36746&r2=36747
> =======================================================================
> =======
> --- trunk/subversion/libsvn_subr/subst.c Mon Mar 23 04:11:03 2009
> (r36746)
> +++ trunk/subversion/libsvn_subr/subst.c Mon Mar 23 12:24:54 2009
> (r36747)
> @@ -1241,6 +1241,9 @@ svn_subst_translate_cstring2(const char
> /* Jam the text into the destination stream (to translate it). */
> SVN_ERR(svn_stream_write(dst_stream, src, &len));
>
> + /* Close the destination stream to flush unwritten data. */
> + SVN_ERR(svn_stream_close(dst_stream));
> +
> *dst = dst_stringbuf->data;
> return SVN_NO_ERROR;
> }
Hi,
How could this cause unflushed data?
If I look at svn_stream_from_stringbuf() it doesn't define an explicit close
function, so the svn_stream_close() will be a no-op.
(write_handler_stringbuf in stream.c directly adds all data into the string
buffer; there is no extra buffer layer)
Bert
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=1391896
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1406299
RE: svn commit: r36747 - trunk/subversion/libsvn_subr
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: Tuesday, March 24, 2009 11:18 PM
> To: dev@subversion.tigris.org
> Subject: Re: svn commit: r36747 - trunk/subversion/libsvn_subr
>
> I think the change is fine. "Knowing" that a close is a no-op implies
> knowing the implementation, rather than the abstraction.
>
> If you can demonstrate this close is a problem (e.g. performance),
> then we can eliminate it, leaving a comment in place to explain the
> situation.
No.. I just read it as a no-op that didn't need backporting.. But rechecking the code I found the reason that it needs the close. (The second wrapping of the stream).
I'll add my +1 on the backport nomination.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1406948
Re: svn commit: r36747 - trunk/subversion/libsvn_subr
Posted by Greg Stein <gs...@gmail.com>.
I think the change is fine. "Knowing" that a close is a no-op implies
knowing the implementation, rather than the abstraction.
If you can demonstrate this close is a problem (e.g. performance),
then we can eliminate it, leaving a comment in place to explain the
situation.
Cheers,
-g
On Tue, Mar 24, 2009 at 22:35, Bert Huijben <rh...@sharpsvn.net> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:chemodax@gmail.com]
>> Sent: Monday, March 23, 2009 8:25 PM
>> To: svn@subversion.tigris.org
>> Subject: svn commit: r36747 - trunk/subversion/libsvn_subr
>>
>> Author: zhakov
>> Date: Mon Mar 23 12:24:54 2009
>> New Revision: 36747
>>
>> Log:
>> * subversion/libsvn_subr/subst.c:
>> (svn_subst_translate_cstring2): Close the destination stream to flush
>> unwritten data.
>>
>> Modified:
>> trunk/subversion/libsvn_subr/subst.c
>>
>> Modified: trunk/subversion/libsvn_subr/subst.c
>> URL:
>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/subst.c?p
>> athrev=36747&r1=36746&r2=36747
>> =======================================================================
>> =======
>> --- trunk/subversion/libsvn_subr/subst.c Mon Mar 23 04:11:03 2009
>> (r36746)
>> +++ trunk/subversion/libsvn_subr/subst.c Mon Mar 23 12:24:54 2009
>> (r36747)
>> @@ -1241,6 +1241,9 @@ svn_subst_translate_cstring2(const char
>> /* Jam the text into the destination stream (to translate it). */
>> SVN_ERR(svn_stream_write(dst_stream, src, &len));
>>
>> + /* Close the destination stream to flush unwritten data. */
>> + SVN_ERR(svn_stream_close(dst_stream));
>> +
>> *dst = dst_stringbuf->data;
>> return SVN_NO_ERROR;
>> }
>
> Hi,
>
> How could this cause unflushed data?
>
> If I look at svn_stream_from_stringbuf() it doesn't define an explicit close
> function, so the svn_stream_close() will be a no-op.
>
> (write_handler_stringbuf in stream.c directly adds all data into the string
> buffer; there is no extra buffer layer)
>
> Bert
>
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
>> d=1391896
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1406301
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1406546