You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Marcus Comstedt <ma...@mc.pp.se> on 2003/02/28 15:23:01 UTC
[PATCH] NULL-handling problem with libsvn_subr/config
Well, I seem to be in my usual bug-trigging form. :-)
The find_option() function i libsvn_subr/config.c segfaults if it gets
a NULL cfg parameter. This is bad, since it is used from
svn_config_enumerate(), which is used from svn_config_find_group(),
which is supposed to handle a NULL cfg parameter. At least, that's
what libsvn_ra_dav/session.c thinks. In case svn_config_find_group()
is _supposed_ to segfault on a NULL cfg, I have an alternative patch
that fixes libsvn_ra_dav/session.c instead. It is not necessary to
apply both.
---8<--- patch variant 1: Let svn_config_find_group (and
svn_config_enumerate and find_option) accept a NULL cfg ---8<---
Index: subversion/libsvn_subr/config.c
===================================================================
--- subversion/libsvn_subr/config.c (revision 5152)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -349,12 +349,17 @@
{
void *sec_ptr;
- /* Canonicalize the hash key */
- svn_stringbuf_set (cfg->tmp_key, section);
- make_hash_key (cfg->tmp_key->data);
+ if (cfg) {
+ /* Canonicalize the hash key */
+ svn_stringbuf_set (cfg->tmp_key, section);
+ make_hash_key (cfg->tmp_key->data);
- sec_ptr = apr_hash_get (cfg->sections, cfg->tmp_key->data,
- cfg->tmp_key->len);
+ sec_ptr = apr_hash_get (cfg->sections, cfg->tmp_key->data,
+ cfg->tmp_key->len);
+ }
+ else
+ sec_ptr = NULL;
+
if (sectionp != NULL)
*sectionp = sec_ptr;
---8<--- patch variant 2: make get_server_settings not assume that
svn_config_find_group can handle NULL ---8<---
Index: subversion/libsvn_ra_dav/session.c
===================================================================
--- subversion/libsvn_ra_dav/session.c (revision 5152)
+++ subversion/libsvn_ra_dav/session.c (working copy)
@@ -176,8 +176,12 @@
SVN_CONFIG_OPTION_NEON_DEBUG_MASK, NULL);
}
- server_group = svn_config_find_group(cfg, requested_host,
- SVN_CONFIG_SECTION_GROUPS,
pool);
+ if (cfg)
+ server_group = svn_config_find_group(cfg, requested_host,
+ SVN_CONFIG_SECTION_GROUPS,
pool);
+ else
+ server_group = NULL;
+
if (server_group)
{
svn_config_get(cfg, proxy_host, server_group,
---8<---
// Marcus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by cm...@collab.net.
Marcus Comstedt <ma...@mc.pp.se> writes:
> > Check your formatting here, too. SVN_CONFIG_SECTION_GROUPS looks like
> > it should be shifted one column to the right.
>
> That, on the other hand, is an optical illusion caused by TABs. The
> indentation becomes correct if you apply the patch (unless you do
> TAB->space conversion on the patch first).
Ah... we don't do tabs in Subversion sourcecode. :-)
Mind banging out a new, clean, whitespace-goodness-having patch?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Marcus Comstedt <ma...@mc.pp.se>.
cmpilato@collab.net writes:
> I'm *sure* you meant:
>
> if (cfg)
> {
> /* Canonicalize ...
Yes. I actually carefully wrote it like that at first, but in
svn_config_find_group. But when I moved the check down to find_option
in order to maximize the number of birds my stone would kill, I forgot
to add the extra whitespace...
> Check your formatting here, too. SVN_CONFIG_SECTION_GROUPS looks like
> it should be shifted one column to the right.
That, on the other hand, is an optical illusion caused by TABs. The
indentation becomes correct if you apply the patch (unless you do
TAB->space conversion on the patch first).
// Marcus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by cm...@collab.net.
Marcus Comstedt <ma...@mc.pp.se> writes:
> Well, I seem to be in my usual bug-trigging form. :-)
>
> The find_option() function i libsvn_subr/config.c segfaults if it gets
> a NULL cfg parameter. This is bad, since it is used from
> svn_config_enumerate(), which is used from svn_config_find_group(),
> which is supposed to handle a NULL cfg parameter. At least, that's
> what libsvn_ra_dav/session.c thinks. In case svn_config_find_group()
> is _supposed_ to segfault on a NULL cfg, I have an alternative patch
> that fixes libsvn_ra_dav/session.c instead. It is not necessary to
> apply both.
>
>
> ---8<--- patch variant 1: Let svn_config_find_group (and
> svn_config_enumerate and find_option) accept a NULL cfg ---8<---
>
> Index: subversion/libsvn_subr/config.c
> ===================================================================
> --- subversion/libsvn_subr/config.c (revision 5152)
> +++ subversion/libsvn_subr/config.c (working copy)
> @@ -349,12 +349,17 @@
> {
> void *sec_ptr;
>
> - /* Canonicalize the hash key */
> - svn_stringbuf_set (cfg->tmp_key, section);
> - make_hash_key (cfg->tmp_key->data);
> + if (cfg) {
> + /* Canonicalize the hash key */
> + svn_stringbuf_set (cfg->tmp_key, section);
> + make_hash_key (cfg->tmp_key->data);
I'm *sure* you meant:
if (cfg)
{
/* Canonicalize ...
> Index: subversion/libsvn_ra_dav/session.c
> ===================================================================
> --- subversion/libsvn_ra_dav/session.c (revision 5152)
> +++ subversion/libsvn_ra_dav/session.c (working copy)
> @@ -176,8 +176,12 @@
> SVN_CONFIG_OPTION_NEON_DEBUG_MASK, NULL);
> }
>
> - server_group = svn_config_find_group(cfg, requested_host,
> - SVN_CONFIG_SECTION_GROUPS,
> pool);
> + if (cfg)
> + server_group = svn_config_find_group(cfg, requested_host,
> + SVN_CONFIG_SECTION_GROUPS,
> pool);
> + else
> + server_group = NULL;
> +
Check your formatting here, too. SVN_CONFIG_SECTION_GROUPS looks like
it should be shifted one column to the right.
--C-Mike, throwing out good code on formatting violations since his
conception.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Marcus Comstedt <ma...@mc.pp.se>.
Michael Wood <mw...@its.uct.ac.za> writes:
> Sorry to annoy you. That wasn't my intention. I was just pointing out
> that the patch was wrapped by your mail client.
>
> I have no say in what gets accepted or rejected and I have no commit
> access. You can ignore me completely if you like.
Don't worry, I wasn't really as annoyed as I sounded. Greg Stein has
a point in that a certain amount of care should be taken in how
contributions are recieved though, not everybody is as bighearted
(and modest!) as I am... :-)
// Marcus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Michael Wood <mw...@its.uct.ac.za>.
Hi Marcus
On Sat, Mar 01, 2003 at 03:25:24PM +0100, Marcus Comstedt wrote:
>
> Michael Wood <mw...@its.uct.ac.za> writes:
>
> > > Ok. Here it is again, with all TABs replaced with spaces to make Mike
> > > happy, courtesy of M-x untabify...
> > [snip]
> >
> > Now all you need is M-x unwrapify ;)
>
>
> Or I need to stop sending in patches and just let you keep your
> friggin' bugs...
Sorry to annoy you. That wasn't my intention. I was just pointing out
that the patch was wrapped by your mail client.
I have no say in what gets accepted or rejected and I have no commit
access. You can ignore me completely if you like.
--
Michael Wood <mw...@its.uct.ac.za>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Mar 01, 2003 at 03:25:24PM +0100, Marcus Comstedt wrote:
>
> Michael Wood <mw...@its.uct.ac.za> writes:
>
> > > Ok. Here it is again, with all TABs replaced with spaces to make Mike
> > > happy, courtesy of M-x untabify...
> > [snip]
> >
> > Now all you need is M-x unwrapify ;)
>
>
> Or I need to stop sending in patches and just let you keep your
> friggin' bugs...
No doubt.
Guys: there really is a limit to the nitpicking on patches that volunteers
send in. I mean, come on. If they take the time to resend it *three* times
with minor tweaks, then leave it alone already. I can see going around many
times on larger, more complex patches, but Marcus' wasn't one of those.
oof...
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Marcus Comstedt <ma...@mc.pp.se>.
Michael Wood <mw...@its.uct.ac.za> writes:
> > Ok. Here it is again, with all TABs replaced with spaces to make Mike
> > happy, courtesy of M-x untabify...
> [snip]
>
> Now all you need is M-x unwrapify ;)
Or I need to stop sending in patches and just let you keep your
friggin' bugs...
// Marcus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Michael Wood <mw...@its.uct.ac.za>.
On Fri, Feb 28, 2003 at 05:00:21PM +0100, Marcus Comstedt wrote:
>
> Greg Hudson <gh...@MIT.EDU> writes:
>
> > We should never ever add null pointer handling to a function without
> > explicitly noting in its doc string that it handles null pointers.
> >
> > And we shouldn't do it at all unless it makes things significantly more
> > convenient, which in this case it doesn't. So I prefer the session.c
> > patch.
>
> Ok. Here it is again, with all TABs replaced with spaces to make Mike
> happy, courtesy of M-x untabify...
[snip]
Now all you need is M-x unwrapify ;)
--
Michael Wood <mw...@its.uct.ac.za>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Greg Stein <gs...@lyra.org>.
Applied in rev 5162. Thanks!
Cheers,
-g
On Fri, Feb 28, 2003 at 05:00:21PM +0100, Marcus Comstedt wrote:
>
> Greg Hudson <gh...@MIT.EDU> writes:
>
> > We should never ever add null pointer handling to a function without
> > explicitly noting in its doc string that it handles null pointers.
> >
> > And we shouldn't do it at all unless it makes things significantly more
> > convenient, which in this case it doesn't. So I prefer the session.c
> > patch.
>
> Ok. Here it is again, with all TABs replaced with spaces to make Mike
> happy, courtesy of M-x untabify...
>
> Index: subversion/libsvn_ra_dav/session.c
> ===================================================================
> --- subversion/libsvn_ra_dav/session.c (revision 5152)
> +++ subversion/libsvn_ra_dav/session.c (working copy)
> @@ -176,8 +176,12 @@
> SVN_CONFIG_OPTION_NEON_DEBUG_MASK, NULL);
> }
>
> - server_group = svn_config_find_group(cfg, requested_host,
> - SVN_CONFIG_SECTION_GROUPS,
> pool);
> + if (cfg)
> + server_group = svn_config_find_group(cfg, requested_host,
> + SVN_CONFIG_SECTION_GROUPS,
> pool);
> + else
> + server_group = NULL;
> +
> if (server_group)
> {
> svn_config_get(cfg, proxy_host, server_group,
>
>
> // Marcus
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Marcus Comstedt <ma...@mc.pp.se>.
Greg Hudson <gh...@MIT.EDU> writes:
> We should never ever add null pointer handling to a function without
> explicitly noting in its doc string that it handles null pointers.
>
> And we shouldn't do it at all unless it makes things significantly more
> convenient, which in this case it doesn't. So I prefer the session.c
> patch.
Ok. Here it is again, with all TABs replaced with spaces to make Mike
happy, courtesy of M-x untabify...
Index: subversion/libsvn_ra_dav/session.c
===================================================================
--- subversion/libsvn_ra_dav/session.c (revision 5152)
+++ subversion/libsvn_ra_dav/session.c (working copy)
@@ -176,8 +176,12 @@
SVN_CONFIG_OPTION_NEON_DEBUG_MASK, NULL);
}
- server_group = svn_config_find_group(cfg, requested_host,
- SVN_CONFIG_SECTION_GROUPS,
pool);
+ if (cfg)
+ server_group = svn_config_find_group(cfg, requested_host,
+ SVN_CONFIG_SECTION_GROUPS,
pool);
+ else
+ server_group = NULL;
+
if (server_group)
{
svn_config_get(cfg, proxy_host, server_group,
// Marcus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] NULL-handling problem with libsvn_subr/config
Posted by Greg Hudson <gh...@MIT.EDU>.
We should never ever add null pointer handling to a function without
explicitly noting in its doc string that it handles null pointers.
And we shouldn't do it at all unless it makes things significantly more
convenient, which in this case it doesn't. So I prefer the session.c
patch.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org