Re: svn commit: r1394346 - /subversion/trunk/subversion/libsvn_ra_svn/client.c

On Fri, Oct 5, 2012 at 4:03 AM, C. Michael Pilato <cmpilato_at_collab.net>wrote:

> On 10/04/2012 09:55 PM, stefan2_at_apache.org wrote:
> > Author: stefan2
> > Date: Fri Oct 5 01:54:59 2012
> > New Revision: 1394346
> >
> > URL: http://svn.apache.org/viewvc?rev=1394346&view=rev
> > Log:
> > * subversion/libsvn_ra_svn/client.c
> > (ra_svn_commit): initialize *all* members of the ccb struct
> >
> > Modified:
> > subversion/trunk/subversion/libsvn_ra_svn/client.c
> >
> > Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/client.c?rev=1394346&r1=1394345&r2=1394346&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
> > +++ subversion/trunk/subversion/libsvn_ra_svn/client.c Fri Oct 5
> 01:54:59 2012
> > @@ -1044,6 +1044,7 @@ static svn_error_t *ra_svn_commit(svn_ra
> > ccb = apr_palloc(pool, sizeof(*ccb));
> > ccb->sess_baton = sess_baton;
> > ccb->pool = pool;
> > + ccb->new_rev = NULL;
> > ccb->callback = callback;
> > ccb->callback_baton = callback_baton;
>
> We pretty commonly use apr_pcalloc() for exactly this reason.

I'm, aware of that and my original patch had the apr_pcalloc in it.

> Yeah, it may
> cost an extra couple of nanoseconds, but...
>

It's not about the costs. Looking for the root cause of a crash,
I scanned the whole ra_svn -related code for apr_palloc and
found it was used for all baton structures, followed by an
explicit initialization of all elements.

So, I had the option to either implicitly or explicitly set the NULL
pointer and both seemed equally valid.

I went with the explicit assignment.

-- Stefan^2.

-- 
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
 for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*