You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2011/07/13 21:28:18 UTC
svn commit: r1146270 - /subversion/branches/1.7.x/STATUS
Author: cmpilato
Date: Wed Jul 13 19:28:18 2011
New Revision: 1146270
URL: http://svn.apache.org/viewvc?rev=1146270&view=rev
Log:
* STATUS: Cast votes.
Modified:
subversion/branches/1.7.x/STATUS
Modified: subversion/branches/1.7.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.7.x/STATUS?rev=1146270&r1=1146269&r2=1146270&view=diff
==============================================================================
--- subversion/branches/1.7.x/STATUS (original)
+++ subversion/branches/1.7.x/STATUS Wed Jul 13 19:28:18 2011
@@ -21,7 +21,7 @@ Candidate changes:
Improves general usability of the svn client and the libsvn_client api
and would have to wait for 1.8.0 if it doesn't get in 1.7.0.
Votes:
- +1: rhuijben
+ +1: rhuijben, cmpilato
* r1146131, r1146134
Add svn_fs_verify() API.
@@ -31,6 +31,7 @@ Candidate changes:
Votes:
+1: danielsh, rhuijben
-1: stsp (no-op API change)
+ -1: cmpilato (no-op API change -- will reconsider if real utility is added)
* r1146214
Handle NULL inputs when stringifying svn_checksum_t.
@@ -38,6 +39,7 @@ Candidate changes:
Avoids segfaults.
Votes:
+1: danielsh
+ -0: cmpilato (problem is with callers, not implementation)
Candidate changes for 1.7.x (post-1.7.0):
=========================================
Re: svn commit: r1146270 - /subversion/branches/1.7.x/STATUS
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/13/2011 03:40 PM, Daniel Shahaf wrote:
> As I see it, the options are:
>
> * Pull this backport nomination. No svn_fs_verify() in 1.7.x
>
> * Backport it, and backport the implementation in 1.7.x (x >= 1)
>
> * Backport svn_fs_verify() and its implementation in 1.7.0
I see what you see, except I don't see that middle option. Releasing a
to-be-implemented-later API is merely an attempt to skirt our compatibility
guarantees. And in my experience, when an API is introduced so quickly and
without an actual implementation, there's a very good chance that with the
implementation will come the frustrating reality that the API as originally
proposed was ill-suited to the purposes for which it was created.
I'm not maintaining a bias on the other two options, though: I can go
either way right now on "pull the backport nomination" or "also nominate
some actual value for 1.7.0".
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1146270 - /subversion/branches/1.7.x/STATUS
Posted by Daniel Shahaf <da...@apache.org>.
cmpilato@apache.org wrote on Wed, Jul 13, 2011 at 19:28:18 -0000:
> @@ -31,6 +31,7 @@ Candidate changes:
> Votes:
> +1: danielsh, rhuijben
> -1: stsp (no-op API change)
> + -1: cmpilato (no-op API change -- will reconsider if real utility is added)
Well, I've since implemented svn_fs_verify() on trunk; and the theory
is that backporting these two revisions for 1.7.0 is required if we're
to backport the implementation in a future 1.7.x release.
As I see it, the options are:
* Pull this backport nomination. No svn_fs_verify() in 1.7.x
* Backport it, and backport the implementation in 1.7.x (x >= 1)
* Backport svn_fs_verify() and its implementation in 1.7.0
Re: svn_checksum_to_cstring_display() Re: svn commit: r1146270 -
/subversion/branches/1.7.x/STATUS
Posted by Daniel Shahaf <da...@apache.org>.
svn_checksum_to_cstring() was added in 1.6 and learnt to do this NULL check in 1.7.
I've documented the change for svn_checksum_to_cstring_display() and
added that to the STATUS entry.
Bert, Mike: if you're standing by your -0's, please let me know so I can
prepare a 1.7.0 backport patch fixing the call in representation_string().
C. Michael Pilato wrote on Wed, Jul 13, 2011 at 16:04:50 -0400:
> On 07/13/2011 04:00 PM, Daniel Shahaf wrote:
> > C. Michael Pilato wrote on Wed, Jul 13, 2011 at 15:54:31 -0400:
> >> On 07/13/2011 03:46 PM, Daniel Shahaf wrote:
> >>> cmpilato@apache.org wrote on Wed, Jul 13, 2011 at 19:28:18 -0000:
> >>>> * r1146214
> >>>> Handle NULL inputs when stringifying svn_checksum_t.
> >>>> @@ -38,6 +39,7 @@ Candidate changes:
> >>>> Avoids segfaults.
> >>>> Votes:
> >>>> +1: danielsh
> >>>> + -0: cmpilato (problem is with callers, not implementation)
> >>>
> >>> Do you want to convert the 'return NULL;' into an assertion then?
> >>
> >> Why? (I honestly don't see what's motivating any change at all here.) A
> >> segfault in the function because of a NULL pointer deref; a segfault in the
> >> caller because it tries to use what should be a string but is actually a
> >> NULL (despite the docstring not foretelling this behavior, even); an
> >> assert() ... these all look the same to me. :-)
> >
> > SVN_ERR_ASSERT, not assert(). I assume that an SVN_ERR_ASSERT is better
> > than SIGSEGV.
>
> The function doesn't return an svn_error_t *. So you must either rev it, or
> use SVN_ERR_ASSERT_NO_RETURN (which just aborts).
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
Re: svn_checksum_to_cstring_display() Re: svn commit: r1146270 -
/subversion/branches/1.7.x/STATUS
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/13/2011 04:00 PM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Wed, Jul 13, 2011 at 15:54:31 -0400:
>> On 07/13/2011 03:46 PM, Daniel Shahaf wrote:
>>> cmpilato@apache.org wrote on Wed, Jul 13, 2011 at 19:28:18 -0000:
>>>> * r1146214
>>>> Handle NULL inputs when stringifying svn_checksum_t.
>>>> @@ -38,6 +39,7 @@ Candidate changes:
>>>> Avoids segfaults.
>>>> Votes:
>>>> +1: danielsh
>>>> + -0: cmpilato (problem is with callers, not implementation)
>>>
>>> Do you want to convert the 'return NULL;' into an assertion then?
>>
>> Why? (I honestly don't see what's motivating any change at all here.) A
>> segfault in the function because of a NULL pointer deref; a segfault in the
>> caller because it tries to use what should be a string but is actually a
>> NULL (despite the docstring not foretelling this behavior, even); an
>> assert() ... these all look the same to me. :-)
>
> SVN_ERR_ASSERT, not assert(). I assume that an SVN_ERR_ASSERT is better
> than SIGSEGV.
The function doesn't return an svn_error_t *. So you must either rev it, or
use SVN_ERR_ASSERT_NO_RETURN (which just aborts).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn_checksum_to_cstring_display() Re: svn commit: r1146270 -
/subversion/branches/1.7.x/STATUS
Posted by Daniel Shahaf <da...@apache.org>.
C. Michael Pilato wrote on Wed, Jul 13, 2011 at 15:54:31 -0400:
> On 07/13/2011 03:46 PM, Daniel Shahaf wrote:
> > cmpilato@apache.org wrote on Wed, Jul 13, 2011 at 19:28:18 -0000:
> >> * r1146214
> >> Handle NULL inputs when stringifying svn_checksum_t.
> >> @@ -38,6 +39,7 @@ Candidate changes:
> >> Avoids segfaults.
> >> Votes:
> >> +1: danielsh
> >> + -0: cmpilato (problem is with callers, not implementation)
> >
> > Do you want to convert the 'return NULL;' into an assertion then?
>
> Why? (I honestly don't see what's motivating any change at all here.) A
> segfault in the function because of a NULL pointer deref; a segfault in the
> caller because it tries to use what should be a string but is actually a
> NULL (despite the docstring not foretelling this behavior, even); an
> assert() ... these all look the same to me. :-)
SVN_ERR_ASSERT, not assert(). I assume that an SVN_ERR_ASSERT is better
than SIGSEGV.
Re: svn_checksum_to_cstring_display() Re: svn commit: r1146270 -
/subversion/branches/1.7.x/STATUS
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 07/13/2011 03:46 PM, Daniel Shahaf wrote:
> cmpilato@apache.org wrote on Wed, Jul 13, 2011 at 19:28:18 -0000:
>> * r1146214
>> Handle NULL inputs when stringifying svn_checksum_t.
>> @@ -38,6 +39,7 @@ Candidate changes:
>> Avoids segfaults.
>> Votes:
>> +1: danielsh
>> + -0: cmpilato (problem is with callers, not implementation)
>
> Do you want to convert the 'return NULL;' into an assertion then?
Why? (I honestly don't see what's motivating any change at all here.) A
segfault in the function because of a NULL pointer deref; a segfault in the
caller because it tries to use what should be a string but is actually a
NULL (despite the docstring not foretelling this behavior, even); an
assert() ... these all look the same to me. :-)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
svn_checksum_to_cstring_display() Re: svn commit: r1146270 -
/subversion/branches/1.7.x/STATUS
Posted by Daniel Shahaf <da...@apache.org>.
cmpilato@apache.org wrote on Wed, Jul 13, 2011 at 19:28:18 -0000:
> * r1146214
> Handle NULL inputs when stringifying svn_checksum_t.
> @@ -38,6 +39,7 @@ Candidate changes:
> Avoids segfaults.
> Votes:
> +1: danielsh
> + -0: cmpilato (problem is with callers, not implementation)
Do you want to convert the 'return NULL;' into an assertion then?