You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2007/07/04 14:45:39 UTC
Re: svn commit: r25630 - trunk/subversion/libsvn_ra_serf
On 7/3/07, zhakov@tigris.org <zh...@tigris.org> wrote:
> Author: zhakov
> Date: Tue Jul 3 01:21:25 2007
> New Revision: 25630
>
> Log:
> ra_serf: Replace several abort() calls to errors.
>
> * subversion/libsvn_ra_serf/blame.c
> (cdata_blame): Return error from svn_stream_write() if it occur.
> * subversion/libsvn_ra_serf/property.c
> () Include svn_private_config.h for '_()'.
> (end_propfind): Generate error on unknown encoding instead of abort().
>
> Modified:
> trunk/subversion/libsvn_ra_serf/blame.c
> trunk/subversion/libsvn_ra_serf/property.c
>
> Modified: trunk/subversion/libsvn_ra_serf/blame.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/blame.c?pathrev=25630&r1=25629&r2=25630
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/blame.c (original)
> +++ trunk/subversion/libsvn_ra_serf/blame.c Tue Jul 3 01:21:25 2007
> @@ -2,7 +2,7 @@
> * blame.c : entry point for blame RA functions for ra_serf
> *
> * ====================================================================
> - * Copyright (c) 2006 CollabNet. All rights reserved.
> + * Copyright (c) 2006-2007 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -349,11 +349,7 @@
>
> ret_len = len;
>
> - svn_stream_write(info->stream, data, &ret_len);
> - if (ret_len != len)
> - {
> - abort();
> - }
> + SVN_ERR(svn_stream_write(info->stream, data, &ret_len));
> }
Hi Ivan,
The above fixes an error leak, but you also changed the meaning of the
code: it won't stop or error out if (ret_len != len).
Maybe we do need to raise an error (such as "unexpected end of
stream") in that case too?
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r25630 - trunk/subversion/libsvn_ra_serf
Posted by Erik Huelsmann <eh...@gmail.com>.
On 7/4/07, Ivan Zhakov <zh...@tigris.org> wrote:
> On 7/4/07, Erik Huelsmann <eh...@gmail.com> wrote:
> > On 7/3/07, zhakov@tigris.org <zh...@tigris.org> wrote:
> > > Author: zhakov
> > > Date: Tue Jul 3 01:21:25 2007
> > > New Revision: 25630
> > >
> > > Log:
> > > ra_serf: Replace several abort() calls to errors.
> > >
> > > * subversion/libsvn_ra_serf/blame.c
> > > (cdata_blame): Return error from svn_stream_write() if it occur.
> > > * subversion/libsvn_ra_serf/property.c
> > > () Include svn_private_config.h for '_()'.
> > > (end_propfind): Generate error on unknown encoding instead of abort().
> > >
> > > Modified:
> > > trunk/subversion/libsvn_ra_serf/blame.c
> > > trunk/subversion/libsvn_ra_serf/property.c
> > >
> > > Modified: trunk/subversion/libsvn_ra_serf/blame.c
> > > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/blame.c?pathrev=25630&r1=25629&r2=25630
> > > ==============================================================================
> > > --- trunk/subversion/libsvn_ra_serf/blame.c (original)
> > > +++ trunk/subversion/libsvn_ra_serf/blame.c Tue Jul 3 01:21:25 2007
> > > @@ -2,7 +2,7 @@
> > > * blame.c : entry point for blame RA functions for ra_serf
> > > *
> > > * ====================================================================
> > > - * Copyright (c) 2006 CollabNet. All rights reserved.
> > > + * Copyright (c) 2006-2007 CollabNet. All rights reserved.
> > > *
> > > * This software is licensed as described in the file COPYING, which
> > > * you should have received as part of this distribution. The terms
> > > @@ -349,11 +349,7 @@
> > >
> > > ret_len = len;
> > >
> > > - svn_stream_write(info->stream, data, &ret_len);
> > > - if (ret_len != len)
> > > - {
> > > - abort();
> > > - }
> > > + SVN_ERR(svn_stream_write(info->stream, data, &ret_len));
> > > }
> >
> > Hi Ivan,
> >
> > The above fixes an error leak, but you also changed the meaning of the
> > code: it won't stop or error out if (ret_len != len).
> >
> > Maybe we do need to raise an error (such as "unexpected end of
> > stream") in that case too?
> >
> Hi Erik,
> I had the same idea, but according svn_stream_t documentation that's
> cannot occur without raising error:
> [[
> * The read and write handlers accept length arguments via pointer.
> * On entry to the handler, the pointed-to value should be the amount
> * of data which can be read or the amount of data to write. When the
> * handler returns, the value is reset to the amount of data actually
> * read or written. Handlers are obliged to complete a read or write
> * to the maximum extent possible; thus, a short read with no
> * associated error implies the end of the input stream, and a short
> * write should never occur without an associated error.
> ]]
>
> May be we need comment or debug check. Personally I don't like idea to
> check everything which cannot be happen by interface agreement.
Sorry for the late reaction, I was unavailable for 2 weeks. Reading
your explanation, I think it's fine as it is.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r25630 - trunk/subversion/libsvn_ra_serf
Posted by Ivan Zhakov <zh...@tigris.org>.
On 7/4/07, Erik Huelsmann <eh...@gmail.com> wrote:
> On 7/3/07, zhakov@tigris.org <zh...@tigris.org> wrote:
> > Author: zhakov
> > Date: Tue Jul 3 01:21:25 2007
> > New Revision: 25630
> >
> > Log:
> > ra_serf: Replace several abort() calls to errors.
> >
> > * subversion/libsvn_ra_serf/blame.c
> > (cdata_blame): Return error from svn_stream_write() if it occur.
> > * subversion/libsvn_ra_serf/property.c
> > () Include svn_private_config.h for '_()'.
> > (end_propfind): Generate error on unknown encoding instead of abort().
> >
> > Modified:
> > trunk/subversion/libsvn_ra_serf/blame.c
> > trunk/subversion/libsvn_ra_serf/property.c
> >
> > Modified: trunk/subversion/libsvn_ra_serf/blame.c
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/blame.c?pathrev=25630&r1=25629&r2=25630
> > ==============================================================================
> > --- trunk/subversion/libsvn_ra_serf/blame.c (original)
> > +++ trunk/subversion/libsvn_ra_serf/blame.c Tue Jul 3 01:21:25 2007
> > @@ -2,7 +2,7 @@
> > * blame.c : entry point for blame RA functions for ra_serf
> > *
> > * ====================================================================
> > - * Copyright (c) 2006 CollabNet. All rights reserved.
> > + * Copyright (c) 2006-2007 CollabNet. All rights reserved.
> > *
> > * This software is licensed as described in the file COPYING, which
> > * you should have received as part of this distribution. The terms
> > @@ -349,11 +349,7 @@
> >
> > ret_len = len;
> >
> > - svn_stream_write(info->stream, data, &ret_len);
> > - if (ret_len != len)
> > - {
> > - abort();
> > - }
> > + SVN_ERR(svn_stream_write(info->stream, data, &ret_len));
> > }
>
> Hi Ivan,
>
> The above fixes an error leak, but you also changed the meaning of the
> code: it won't stop or error out if (ret_len != len).
>
> Maybe we do need to raise an error (such as "unexpected end of
> stream") in that case too?
>
Hi Erik,
I had the same idea, but according svn_stream_t documentation that's
cannot occur without raising error:
[[
* The read and write handlers accept length arguments via pointer.
* On entry to the handler, the pointed-to value should be the amount
* of data which can be read or the amount of data to write. When the
* handler returns, the value is reset to the amount of data actually
* read or written. Handlers are obliged to complete a read or write
* to the maximum extent possible; thus, a short read with no
* associated error implies the end of the input stream, and a short
* write should never occur without an associated error.
]]
May be we need comment or debug check. Personally I don't like idea to
check everything which cannot be happen by interface agreement.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org