[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r34824 - branches/1.5.x

From: Bert Huijben <rhuijben_at_open.collab.net>
Date: Fri, 19 Dec 2008 09:21:21 +0100

> -----Original Message-----
> From: Julian Foad [mailto:julianfoad_at_btopenworld.com]
> Sent: donderdag 18 december 2008 22:54
> To: svn_at_subversion.tigris.org
> Subject: svn commit: r34824 - branches/1.5.x
>
> Author: julianfoad
> Date: Thu Dec 18 13:53:47 2008
> New Revision: 34824
>
> Log:
> * STATUS: Remove my veto and add notes about the resolution.
>
> Modified:
> branches/1.5.x/STATUS
>
> Modified: branches/1.5.x/STATUS
> URL:
> http://svn.collab.net/viewvc/svn/branches/1.5.x/STATUS?pathrev=34824&r1
> =34823&r2=34824
> =======================================================================
> =======
> --- branches/1.5.x/STATUS Thu Dec 18 13:04:49 2008 (r34823)
> +++ branches/1.5.x/STATUS Thu Dec 18 13:53:47 2008 (r34824)
> @@ -21,6 +21,9 @@ Candidate changes for 1.5.5:
> (generally missing their first chunk of bytes) which can result
> in such confusing user scenarios as seeing errors with successful
> (200) status codes.
> + Notes:
> + Simple conflict: use of "SVN_ERR_ASSERT(...)" needs to be
> replaced
> + with "#include <assert.h>" and "assert(...)".

This replacement is /not/ always safe!

E.g. in r34828, which is not proposed for backport:

- if (mb->curr_path)
- svn_stringbuf_appendbytes(mb->curr_path, cdata, nlen);
+ SVN_ERR_ASSERT(mb->curr_path);
+ svn_stringbuf_appendbytes(mb->curr_path, cdata, nlen);

Would be replaced

By an assert(mb->curr_path);

When this is compiled in release mode the preprocessor removes the assert
and

* subversion/libsvn_ra_neon/mergeinfo.c
  (cdata_handler): Change bug-hiding "if"s into bug-revealing assertions.

Would become:

* subversion/libsvn_ra_neon/mergeinfo.c
  (cdata_handler): Change bug-hiding in a fatal crash that would use lose
    user code in embedded scenarios (Subclipse, AnkhSVN, etc.)

Note:
SVN_ERR_ASSERT is /not/ removed by the preprocessor!
It just returns an error in release mode.

In this specific case this assertion would be a check based on network
traffic, so this would also introduce a security issue if backported to 1.5
and a tracable error code in 1.6.

        Bert

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=987247
Received on 2008-12-19 09:50:00 CET

This is an archived mail posted to the Subversion Dev mailing list.