You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/09/03 10:47:06 UTC
Re: svn commit: r990385 -
/subversion/trunk/subversion/libsvn_client/patch.c
stsp@apache.org wrote on Sat, Aug 28, 2010 at 15:49:52 -0000:
> Author: stsp
> Date: Sat Aug 28 15:49:52 2010
> New Revision: 990385
>
> URL: http://svn.apache.org/viewvc?rev=990385&view=rev
> Log:
> * subversion/libsvn_client/patch.c
> (try_stream_write): Remove a question I put into a comment, not realising
> that try_stream_write() is there to catch errors like "disk is full".
>
> Modified:
> subversion/trunk/subversion/libsvn_client/patch.c
>
> Modified: subversion/trunk/subversion/libsvn_client/patch.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=990385&r1=990384&r2=990385&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/patch.c (original)
> +++ subversion/trunk/subversion/libsvn_client/patch.c Sat Aug 28 15:49:52 2010
> @@ -1236,11 +1236,7 @@ get_hunk_info(hunk_info_t **hi, patch_ta
>
> /* Attempt to write LEN bytes of DATA to STREAM, the underlying file
> * of which is at ABSPATH. Fail if not all bytes could be written to
> - * the stream. Do temporary allocations in POOL.
> - * ### stsp: Maybe we can remove this? It's only about checking wether
> - * ### all data was written, but we're usually doing buffered I/O
> - * ### anyway so if the disk or filesystem fails we likely won't
> - * ### see the failure right here. I've never seen this error out. */
> + * the stream. Do temporary allocations in POOL. */
> static svn_error_t *
> try_stream_write(svn_stream_t *stream, const char *abspath,
> const char *data, apr_size_t len, apr_pool_t *pool)
>
>
I agree with your question, actually. Note the svn_stream_write() docs:
722 * The read and write handlers accept length arguments via pointer.
723 * On entry to the handler, the pointed-to value should be the amount
724 * of data which can be read or the amount of data to write. When the
725 * handler returns, the value is reset to the amount of data actually
726 * read or written. Handlers are obliged to complete a read or write
727 * to the maximum extent possible; thus, a short read with no
728 * associated error implies the end of the input stream, and a short
^^^^^^^^^^^
729 * write should never occur without an associated error.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Re: svn commit: r990385 -
/subversion/trunk/subversion/libsvn_client/patch.c
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Sep 03, 2010 at 01:47:06PM +0300, Daniel Shahaf wrote:
> stsp@apache.org wrote on Sat, Aug 28, 2010 at 15:49:52 -0000:
> > Author: stsp
> > Date: Sat Aug 28 15:49:52 2010
> > New Revision: 990385
> >
> > URL: http://svn.apache.org/viewvc?rev=990385&view=rev
> > Log:
> > * subversion/libsvn_client/patch.c
> > (try_stream_write): Remove a question I put into a comment, not realising
> > that try_stream_write() is there to catch errors like "disk is full".
> >
> > Modified:
> > subversion/trunk/subversion/libsvn_client/patch.c
> >
> > Modified: subversion/trunk/subversion/libsvn_client/patch.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=990385&r1=990384&r2=990385&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_client/patch.c (original)
> > +++ subversion/trunk/subversion/libsvn_client/patch.c Sat Aug 28 15:49:52 2010
> > @@ -1236,11 +1236,7 @@ get_hunk_info(hunk_info_t **hi, patch_ta
> >
> > /* Attempt to write LEN bytes of DATA to STREAM, the underlying file
> > * of which is at ABSPATH. Fail if not all bytes could be written to
> > - * the stream. Do temporary allocations in POOL.
> > - * ### stsp: Maybe we can remove this? It's only about checking wether
> > - * ### all data was written, but we're usually doing buffered I/O
> > - * ### anyway so if the disk or filesystem fails we likely won't
> > - * ### see the failure right here. I've never seen this error out. */
> > + * the stream. Do temporary allocations in POOL. */
> > static svn_error_t *
> > try_stream_write(svn_stream_t *stream, const char *abspath,
> > const char *data, apr_size_t len, apr_pool_t *pool)
> >
> >
>
> I agree with your question, actually. Note the svn_stream_write() docs:
>
> 722 * The read and write handlers accept length arguments via pointer.
> 723 * On entry to the handler, the pointed-to value should be the amount
> 724 * of data which can be read or the amount of data to write. When the
> 725 * handler returns, the value is reset to the amount of data actually
> 726 * read or written. Handlers are obliged to complete a read or write
> 727 * to the maximum extent possible; thus, a short read with no
> 728 * associated error implies the end of the input stream, and a short
> ^^^^^^^^^^^
> 729 * write should never occur without an associated error.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Oh, that's interesting. That's a weird API, with an output parameter
that's only interesting when an error occured.
But I guess this means we can really remove the silly try_stream_write()
function.
Thanks,
Stefan