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

Re: [PATCH] Tree-conflicts: do_entry_deletion segfault

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 05 Sep 2008 15:54:07 +0100

On Fri, 2008-09-05 at 04:25 +0200, Neels Hofmeyr wrote:
> Quoting:
> * subversion/libsvn_wc/update_editor.c
> (bump_dir_info): Update a doc-string to allow for a directory to have tree
> conflicts.
> (entry_has_local_mods, check_tree_conflict): New functions.
> (do_entry_deletion): Have the parent's admin access baton passed in by the
> caller. Check for tree conflicts.
> ### Broken when parent_adm_access is NULL.
> I've already posted a fix for this one almost two weeks ago. It wasn't being
> heard much.

That's my fault, Neels. I read your message then, and the earlier ones,
and started to reply, but lost my way. Sorry.

> So I'm posting an update, using today's tree-conflicts branch, compacting my
> explanations. You may refer to the following mail for more (confusing) detail:

Excellent. Thanks.

> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=142154
> Date: Sat, 23 Aug 2008 22:43:11 +0200
> From: Neels Hofmeyr <neels_at_elego.de>
> Subject: Re: Tree-conflicts branch - log message / review
> Two problems coincide:
> 1. Tree conflict detection is skipped when examining the path that was
> specifically given as target. E.g. `svn update A/D/G' means that while G's
> contents are checked for tree conflicts, G itself will not be checked.
> 2. Tree conflict detection segfaults if run in that situation given in point
> 1 above.
> This patch fixes both, plus it tweaks two "unrelated" cmdline test to ignore
> a 'C' marker caused by the fix. The fix itself consists of three parts:
> i) A change made in the tree-conflicts branch is reverted to trunk:
> do_entry_deletion() does not need a PARENT_ADM_ACCESS parameter. It was
> being used to indicate the specific case mentioned in point 1 above, so as
> to be able to skip tree-conflicts detection in that case, oddly enough. Stsp
> has pointed out that this change must have been committed by accident.

Heh. It was me who added that. It wasn't by accident. I was trying to
solve the problem of when the target is the root of the WC and therefore
there is no parent WC directory. However, I didn't properly solve that
problem, and I forgot to mention it in the log message, so we might very
well call it an accident.

> ii) Enable tree-conflicts detection for the case in point 1 above.


> iii) Make check_tree_conflict() handle a missing directory properly, so that
> it reports a reason_missing instead of segfaulting. (The diff looks a little
> confusing there because of indent changes.)

Thanks for explaining each part of your changes.

Committed in r32932.

I made just one tweak: There was a point where you wrote "if
(svn_io_check_path(...) != SVN_NO_ERROR ..." which would leak the error
object if it threw an error. Instead, we have to call the function on
its own, as "SVN_ERR(svn_io_check_path(...));". Any error that it might
return is one that we don't know how to deal with, so it is OK to just
return the error to our caller.

Thanks ever so much for fixing this.

- Julian

> [[[
> Fix segfault when the update editor calls do_entry_deletion() on an explicit
> target of an update.
> Patch by: Neels Hofmeyr <neels_at_elego.de>
> * subversion/libsvn_wc/update_editor.c
> (do_entry_deletion, delete_entry, close_edit): Remove parameter
> PARENT_ADM_ACCESS from do_entry_deletion(), reverting to trunk
> and allowing tree-conflicts detection for update targets.
> (check_tree_conflict): Properly handle a missing directory, reporting
> it as `svn_wc_conflict_reason_missing'.
> * subversion/tests/cmdline/update_tests.py
> (update_deleted_missing_dir, another_hudson_problem): Tweak these tests
> to expect and handle the tree-conflicts that inherently arise.
> ]]]

To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-05 16:54:25 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.