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

RE: svn commit: r26813 - trunk/subversion/libsvn_client

From: Paul Burba <pburba_at_collab.net>
Date: 2007-09-28 14:59:11 CEST

> -----Original Message-----
> From: Daniel Rall [mailto:dlr@collab.net]
> Sent: Thursday, September 27, 2007 7:28 PM
> To: Paul Burba
> Cc: dev@subversion.tigris.org
> Subject: Re: svn commit: r26813 - trunk/subversion/libsvn_client
>
> On Thu, 27 Sep 2007, pburba@tigris.org wrote:
> ...
> > Follow-up to r26803, fix some potential segfaults.
> >
> > * subversion/libsvn_client/merge.c
> > (drive_merge_report_editor): Be consistent with our
> assumption that
> > drive_merge_report_editor may be NULL and check for that.
> > (svn_client_merge_peg3): Initialize children_with_mergeinfo.
> ...
> > --- trunk/subversion/libsvn_client/merge.c (original)
> > +++ trunk/subversion/libsvn_client/merge.c Thu Sep 27 12:38:58 2007
> > @@ -1623,7 +1623,7 @@
> >
> > if (merge_b->target_has_dummy_merge_range)
> > default_start = end;
> > - else if (children_with_mergeinfo)
> > + else if (children_with_mergeinfo &&
> children_with_mergeinfo->nelts
> > + > 0)
> > {
> > svn_client__merge_path_t *target_merge_path_t;
> > target_merge_path_t =
> APR_ARRAY_IDX(children_with_mergeinfo, 0,
> > @@ -3797,6 +3797,8 @@
> > }
> > else
> > {
> > + children_with_mergeinfo =
> > + apr_array_make(pool, 0,
> sizeof(svn_client__merge_path_t
> > + *));
> > SVN_ERR(do_merge(URL1,
> > range.start,
> > URL2,
>
> Hey Paul, why was this second case necesssary (e.g. where was
> the segfault)? drive_merge_report_editor() seems to handle a
> NULL hash okay,

Hi Dan,

It didn't at the time, line 1629 in drive_merge_report_editor(),

      target_merge_path_t = APR_ARRAY_IDX(children_with_mergeinfo, 0,
                                          svn_client__merge_path_t *);

would operate on an uninitialized children_with_mergeinfo array in the
case I described here: http://svn.haxx.se/dev/archive-2007-09/0706.shtml

To your point, with the first fix above that couldn't happen any longer.
I was only being overly cautious since the same check was made later on
in the function. Anyhow, as I'm sure you saw, Kamesh addressed all of
this in r26822, so it's a moot point now.

Paul

> as does cleanup_noop_merge(). I didn't catch
> any other consumers of this hash. It'd be nice to drop the
> NULL initialization when declaring children_with_mergeinfo in
> svn_client_merge3(); it's already absent in svn_client_merge_peg3().
>
> Thanks, Dan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Sep 28 15:03:10 2007

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