You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by George Gugulea <gu...@gmail.com> on 2012/08/28 18:08:10 UTC
[PATCH] fix a crash in libsvn_wc
[[[
Used from the kdevelop subversion plugin, the db pointers comes NULL. This
should be treated one way or another, I choose to return error and not an
assert because it might be a recoverable error. This should be reviewed and
an appropriate action should be taken.
]]]
--
gg
Re: [PATCH] fix a crash in libsvn_wc
Posted by Philip Martin <ph...@wandisco.com>.
George Gugulea <gu...@gmail.com> writes:
> [[[
> Used from the kdevelop subversion plugin, the db pointers comes NULL. This
> should be treated one way or another, I choose to return error and not an
> assert because it might be a recoverable error. This should be reviewed and
> an appropriate action should be taken.
> ]]]
>
> attached is the patch for the last svn revision (1378109) of the file
> --
> gg
>
> Index: wc_db_wcroot.c
> ===================================================================
> --- wc_db_wcroot.c (revision 1378109)
> +++ wc_db_wcroot.c (working copy)
> @@ -374,6 +374,17 @@
> int wc_format = 0;
> const char *adm_relpath;
>
> + /* gg:
> + Test the db pointer if NULL: assert if that's ok or take some acction
> + (return an error)
> + For some reason kdevelop subversion module crashes here because of that
> + */
> +
> + if (db == NULL)
> + {
> + return svn_error_createf(SVN_ERR_WC_NOT_WORKING_COPY, NULL, NULL);
> + }
> +
> /* ### we need more logic for finding the database (if it is located
> ### outside of the wcroot) and then managing all of that within DB.
> ### for now: play quick & dirty. */
Does the plugin call svn_client_create_context or svn_wc_context_create?
It must do one or the other, if it simply allocates a structure and sets
the fields to zero then the plugin has a bug (rapidsvn had the same
bug).
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [PATCH] fix a crash in libsvn_wc
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Philip Martin wrote on Wed, Aug 29, 2012 at 15:50:08 +0100:
> George Gugulea <gu...@gmail.com> writes:
>
> > does this function exist in subversion-1.7.6 ?
> >>
> >> svn_client_ctx_t *ctx;
> >> svn_error_t *err;
> >>
> >> err = svn_client_create_context(&ctx, pool)
>
> Yes, it exists in all Subversion versions back to 1.0.
Any function that does _not_ have @since in its doxygen exists in every
released ≥1.0.0 Subversion.
Re: [PATCH] fix a crash in libsvn_wc
Posted by Philip Martin <ph...@wandisco.com>.
George Gugulea <gu...@gmail.com> writes:
> does this function exist in subversion-1.7.6 ?
>>
>> svn_client_ctx_t *ctx;
>> svn_error_t *err;
>>
>> err = svn_client_create_context(&ctx, pool)
Yes, it exists in all Subversion versions back to 1.0.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: [PATCH] fix a crash in libsvn_wc
Posted by George Gugulea <gu...@gmail.com>.
does this function exist in subversion-1.7.6 ?
On Wed, Aug 29, 2012 at 5:08 PM, C. Michael Pilato <cm...@collab.net>wrote:
> On 08/29/2012 09:47 AM, George Gugulea wrote:
> > (sorry, for me subversion is code at-first-sight)
> >
> > in the plugin it seems that this code happens:
> > //======================================================
> > svn_client_ctx_t ctx;
> > memset(&ctx, 0, sizeof(ctx));
> >
> > // get the config based on the configDir passed in
> > svn_config_get_config(&ctx.config, c_configDir, pool);
> >
> > // tell the auth functions where the config is
> > svn_auth_set_parameter(ab, SVN_AUTH_PARAM_CONFIG_DIR,
> > c_configDir);
> >
> > ctx.auth_baton = ab;
> > ctx.log_msg_func = onLogMsg;
> > ctx.log_msg_baton = this;
> > ctx.notify_func = onNotify;
> > ctx.notify_baton = this;
> > ctx.cancel_func = onCancel;
> > ctx.cancel_baton = this;
> >
> > #if (SVN_VER_MAJOR >= 1) && (SVN_VER_MINOR >= 2)
> > ctx.notify_func2 = onNotify2;
> > ctx.notify_baton2 = this;
> > #endif
> > //======================================================
> >
> >
> > so missing from here, as I understand is svn_client_create_context
>
> Exactly. You'll want to begin with:
>
> svn_client_ctx_t *ctx;
> svn_error_t *err;
>
> err = svn_client_create_context(&ctx, pool)
>
> ...and then of course you'll want to handle a non-NULL 'err' in the way
> that
> makes sense for your program (I can't tell from the context provided).
> Obviously, *don't* memset the 'ctx' structure you just created/initialized.
> Then carry on populating bits of your 'ctx' structure pretty much as you
> are already.
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Enterprise Cloud Development
>
>
--
gg
Re: [PATCH] fix a crash in libsvn_wc
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/29/2012 09:47 AM, George Gugulea wrote:
> (sorry, for me subversion is code at-first-sight)
>
> in the plugin it seems that this code happens:
> //======================================================
> svn_client_ctx_t ctx;
> memset(&ctx, 0, sizeof(ctx));
>
> // get the config based on the configDir passed in
> svn_config_get_config(&ctx.config, c_configDir, pool);
>
> // tell the auth functions where the config is
> svn_auth_set_parameter(ab, SVN_AUTH_PARAM_CONFIG_DIR,
> c_configDir);
>
> ctx.auth_baton = ab;
> ctx.log_msg_func = onLogMsg;
> ctx.log_msg_baton = this;
> ctx.notify_func = onNotify;
> ctx.notify_baton = this;
> ctx.cancel_func = onCancel;
> ctx.cancel_baton = this;
>
> #if (SVN_VER_MAJOR >= 1) && (SVN_VER_MINOR >= 2)
> ctx.notify_func2 = onNotify2;
> ctx.notify_baton2 = this;
> #endif
> //======================================================
>
>
> so missing from here, as I understand is svn_client_create_context
Exactly. You'll want to begin with:
svn_client_ctx_t *ctx;
svn_error_t *err;
err = svn_client_create_context(&ctx, pool)
...and then of course you'll want to handle a non-NULL 'err' in the way that
makes sense for your program (I can't tell from the context provided).
Obviously, *don't* memset the 'ctx' structure you just created/initialized.
Then carry on populating bits of your 'ctx' structure pretty much as you
are already.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [PATCH] fix a crash in libsvn_wc
Posted by George Gugulea <gu...@gmail.com>.
(sorry, for me subversion is code at-first-sight)
in the plugin it seems that this code happens:
//======================================================
svn_client_ctx_t ctx;
memset(&ctx, 0, sizeof(ctx));
// get the config based on the configDir passed in
svn_config_get_config(&ctx.config, c_configDir, pool);
// tell the auth functions where the config is
svn_auth_set_parameter(ab, SVN_AUTH_PARAM_CONFIG_DIR,
c_configDir);
ctx.auth_baton = ab;
ctx.log_msg_func = onLogMsg;
ctx.log_msg_baton = this;
ctx.notify_func = onNotify;
ctx.notify_baton = this;
ctx.cancel_func = onCancel;
ctx.cancel_baton = this;
#if (SVN_VER_MAJOR >= 1) && (SVN_VER_MINOR >= 2)
ctx.notify_func2 = onNotify2;
ctx.notify_baton2 = this;
#endif
//======================================================
so missing from here, as I understand is svn_client_create_context
-----------------------------------------------------------------------------------------------------
following is the backtrace:
#0 0xaeae5081 in svn_wc__db_wcroot_parse_local_abspath (wcroot=0xae569d18,
local_relpath=0xae569d1c, db=0x0, local_abspath=
0xaec4e508 "/home/georgeg/Projects/TestKdev4.1",
result_pool=0xaec44018, scratch_pool=0xaec44018)
at subversion/libsvn_wc/wc_db_wcroot.c:383
#1 0xaeadb001 in svn_wc__db_read_info (status=0xae569dd8, kind=0xae569dd0,
revision=0x0, repos_relpath=0x0, repos_root_url=0x0,
repos_uuid=0x0, changed_rev=0x0, changed_date=0x0, changed_author=0x0,
depth=0x0, checksum=0x0, target=0x0, original_repos_relpath=
0x0, original_root_url=0x0, original_uuid=0x0, original_revision=0x0,
lock=0x0, recorded_size=0x0, recorded_mod_time=0x0, changelist=
0x0, conflicted=0x0, op_root=0x0, have_props=0x0, props_mod=0x0,
have_base=0x0, have_more_work=0x0, have_work=0x0, db=0x0,
local_abspath=0xaec4e508 "/home/georgeg/Projects/TestKdev4.1",
result_pool=0xaec44018, scratch_pool=0xaec44018)
at subversion/libsvn_wc/wc_db.c:6764
#2 0xaeaa80f5 in svn_wc__internal_walk_children (db=0x0,
local_abspath=0xaec4e508 "/home/georgeg/Projects/TestKdev4.1",
show_hidden=0,
changelist_filter=0x0, walk_callback=0xaeaa09a0
<info_found_node_callback>, walk_baton=0xae569e50,
walk_depth=svn_depth_empty,
cancel_func=0xaecafb50 <svn::Context::Data::onCancel(void*)>,
cancel_baton=0x844e658, scratch_pool=0xaec44018)
at subversion/libsvn_wc/node.c:697
#3 0xaeaa182b in svn_wc__get_info (wc_ctx=0xb69789dc,
local_abspath=0xaec4e508 "/home/georgeg/Projects/TestKdev4.1", depth=
svn_depth_empty, fetch_excluded=0, fetch_actual_only=1,
changelist_filter=0x0, receiver=0xaec1b160 <wc_info_receiver>,
receiver_baton=
0xae569f2c, cancel_func=0xaecafb50
<svn::Context::Data::onCancel(void*)>, cancel_baton=0x844e658,
scratch_pool=0xaec4e018)
at subversion/libsvn_wc/info.c:453
#4 0xaec1b7e5 in svn_client_info3 (abspath_or_url=0xaec4e508
"/home/georgeg/Projects/TestKdev4.1", peg_revision=0xaecd3868, revision=
0xaecd3868, depth=svn_depth_empty, fetch_excluded=0,
fetch_actual_only=1, changelists=0x0, receiver=
0xaec0f0f0 <info_receiver_relpath_wrapper>, receiver_baton=0xae569fc8,
ctx=0x844e674, pool=0xaec4e018)
at subversion/libsvn_client/info.c:299
#5 0xaec1284e in svn_client_info2 (path_or_url=0xadc01ad4
"/home/georgeg/Projects/TestKdev4.1", peg_revision=0xaecd3868, revision=
0xaecd3868, receiver=0xaecadfd0 <svn::infoReceiverFunc(void*, char
const*, svn_info_t const*, apr_pool_t*)>, receiver_baton=
0xae56a14c, depth=svn_depth_empty, changelists=0x0, ctx=0x844e674,
pool=0xaec4e018) at subversion/libsvn_client/deprecated.c:2343
#6 0xaec12903 in svn_client_info (path_or_url=0xadc01ad4
"/home/georgeg/Projects/TestKdev4.1", peg_revision=0xaecd3868, revision=
0xaecd3868, receiver=0xaecadfd0 <svn::infoReceiverFunc(void*, char
const*, svn_info_t const*, apr_pool_t*)>, receiver_baton=
0xae56a14c, recurse=0, ctx=0x844e674, pool=0xaec4e018) at
subversion/libsvn_client/deprecated.c:2367
#7 0xaecace37 in svn::Client::info (this=0xae56a158, pathOrUrl=...,
recurse=false, revision=..., pegRevision=...)
at
/usr/src/debug/kdevplatform-1.2.3/plugins/subversion/kdevsvncpp/client_status.cpp:404
On Wed, Aug 29, 2012 at 2:00 PM, Bert Huijben <be...@qqmail.nl> wrote:
> The db pointer comes directly from svn_wc_context_t, which should come
> directly from svn_client_ctx_t, where it is always added on proper
> initialization using the constructor function****
>
> ** **
>
> So while I agree that it should not crash, there is no way that any code
> should reach this point without some calling problem.****
>
> ** **
>
> Are you sure you create the svn_client_ctx_t using the constructor
> function?****
>
> ** **
>
> (We had some earlier issues where users just created the struct and
> assumed that initializing all the 1.6 members was enough to get 1.7
> working… For 1.7 this is not enough)****
>
> ** **
>
> Bert****
>
> ** **
>
> *From:* George Gugulea [mailto:gugulea@gmail.com]
> *Sent:* dinsdag 28 augustus 2012 18:10
> *To:* dev@subversion.apache.org
> *Subject:* Re: [PATCH] fix a crash in libsvn_wc****
>
> ** **
>
> [[[****
>
> Used from the kdevelop subversion plugin, the db pointers comes NULL. This
> should be treated one way or another, I choose to return error and not an
> assert because it might be a recoverable error. This should be reviewed and
> an appropriate action should be taken.****
>
> ]]]****
>
> ** **
>
> attached is the patch for the last svn revision (1378109) of the file****
>
> --
> gg****
>
--
gg
RE: [PATCH] fix a crash in libsvn_wc
Posted by Bert Huijben <be...@qqmail.nl>.
The db pointer comes directly from svn_wc_context_t, which should come
directly from svn_client_ctx_t, where it is always added on proper
initialization using the constructor function
So while I agree that it should not crash, there is no way that any code
should reach this point without some calling problem.
Are you sure you create the svn_client_ctx_t using the constructor function?
(We had some earlier issues where users just created the struct and assumed
that initializing all the 1.6 members was enough to get 1.7 working. For 1.7
this is not enough)
Bert
From: George Gugulea [mailto:gugulea@gmail.com]
Sent: dinsdag 28 augustus 2012 18:10
To: dev@subversion.apache.org
Subject: Re: [PATCH] fix a crash in libsvn_wc
[[[
Used from the kdevelop subversion plugin, the db pointers comes NULL. This
should be treated one way or another, I choose to return error and not an
assert because it might be a recoverable error. This should be reviewed and
an appropriate action should be taken.
]]]
attached is the patch for the last svn revision (1378109) of the file
--
gg
Re: [PATCH] fix a crash in libsvn_wc
Posted by George Gugulea <gu...@gmail.com>.
[[[
Used from the kdevelop subversion plugin, the db pointers comes NULL. This
should be treated one way or another, I choose to return error and not an
assert because it might be a recoverable error. This should be reviewed and
an appropriate action should be taken.
]]]
attached is the patch for the last svn revision (1378109) of the file
--
gg