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

[PATCH] error handling for build_text_conflict_resolve_items

From: Stefan <luke1410_at_posteo.de>
Date: Mon, 1 Feb 2016 01:25:43 +0100

Hi,

> Hi,
>
> I've investigated the crash dump provided on the users mailing list in
> the thread: "Subversion crash report".
>
> Not sure what exactly the underlying issue is, but doing a code review
> suggests that SVN is making use of a nullptr without properly checking
> that case.
> starting in libsvn_wc/conflicts.c: build_text_conflict_resolve_items():
> 1. The call to svn_wc__conflict_read_text_conflict() sets mine_abspath
> to null without returning an error (I assume this is correct behavior
> here). Therefore, it looks like m->is_atom is false in this case
> (inside svn_wc__conflict_read_text_conflict()).
Looking deeper in the code, suggests that this means that there simply
was no mine_abspath in this case to resolve (looking at the added the
corresponding data: svn_wc__conflict_skel_add_text_conflict()).

> [...]
> I take it that build_text_conflict_resolve_items() is missing a
> null-check for mine_abspath() in the switch case before calling
> merge_showing_conflicts() (according to what that function does, if
> install_from_abspath is null), but then I take it that you are more
> familiar with the intended behavior here (maybe the missing null-check
> on Windows would be sufficient to restore the originally intended
> behavior?).
Hence I take it that the original conclusion was correct and there
should simply be an error check being added to
build_text_conflict_resolve_items().

Attached is a suggested patch with that added error check (it's fully
untested, since I've got no idea how that case is triggered).
[[
Prevent user reported crash upon conflict resolution with missing
mine_abspath.
See:
http://mail-archives.apache.org/mod_mbox/subversion-users/201601.mbox/%3C080F6E74ACB8E84E9FC333A7AB0BFFFB7AF7C020%40OIT-TEAQEXMBX01.som.w2k.state.me.us%3E

Reported by: James Patten <James.Patten_at_maine.gov>

* libsvn_wc/conflicts.c:
    (build_text_conflict_resolve_items): check mine_abspath for null before
                                         using it and return an error
instead.
]]

Regards,
Stefan

  • text/x-diff attachment: stored
Received on 2016-02-01 01:26:11 CET

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