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 <da...@apache.org> on 2011/07/13 21:46:19 UTC
svn_checksum_to_cstring_display() Re: svn commit: r1146270 -
/subversion/branches/1.7.x/STATUS
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?
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