Gerald Macinenti <gerald.macinenti_at_letitwave.fr> writes:
> Here is a simple patch which seams to work for us in order to use
> wildcards in authz access file paths.
>
> I've seen the patch by Rodrigo Gallardo but frankly didn't understand
> all the details (I'm not a Subversion nor a C expert sorry ;)), but I
> think mine takes the problem at a lower level, or maybe is it just to
> naive?
>
> I basically defined a find_section function to replace find_option in
> svn_config_enumerate2 and used apr_fnmatch to get the wildcard
> matching of configuration sections.
>
> I didn't write any specific test yet but existing tests passed well.
>
> I submit it to get some feedback/advises about it and then come back
> with a better one if it can be useful.
Thanks. One thing that would help would be a log message; see
http://subversion.tigris.org/hacking.html#patches for more on submitting
patches.
It's great that you documented the new function; people often overlook
that. The documentation doesn't state what happens in the case of
multiple matches, though. Since the sections are in a hash table, my
guess is that a wildcard with multiple matches will behave unpredictably
-- there's no way to know which one it will match. I'm not saying this
is necessarily a bug, but it should be documented.
I haven't had time to review your patch beyond the above, but I've
linked to your message from issue #2662.
-Karl
> --- subversion/libsvn_subr/config.c.orig
> +++ subversion/libsvn_subr/config.c
> @@ -24,6 +24,7 @@
>
> #include <apr_general.h>
> #include <apr_lib.h>
> +#include <apr_fnmatch.h>
> #include "svn_error.h"
> #include "svn_pools.h"
> #include "config_impl.h"
> @@ -356,6 +357,38 @@
> }
>
>
> +/* Find a matching section in the configuration with wildcard support
> + Return a pointer to a section in CFG, or NULL if it doesn't exist. */
> +static cfg_section_t *
> +find_section(svn_config_t *cfg, const char *section, apr_pool_t *pool)
> +{
> + apr_hash_index_t *sec_ndx;
> + apr_pool_t *iteration_pool;
> + cfg_section_t * sectionp = NULL;
> + int count = 0;
> +
> + iteration_pool = svn_pool_create(pool);
> + for (sec_ndx = apr_hash_first(iteration_pool, cfg->sections);
> + sec_ndx != NULL;
> + sec_ndx = apr_hash_next(sec_ndx))
> + {
> + void *sec_ptr;
> + const void *sec_name;
> +
> + apr_hash_this(sec_ndx, &sec_name, NULL, &sec_ptr);
> + if (apr_fnmatch(sec_name, section, APR_FNM_PATHNAME | APR_FNM_CASE_BLIND) == APR_SUCCESS) {
> + sectionp = sec_ptr;
> + break;
> + }
> + ++count;
> + svn_pool_clear(iteration_pool);
> + }
> + svn_pool_destroy(iteration_pool);
> +
> + return sectionp;
> +}
> +
> +
> /* Return a pointer to an option in CFG, or NULL if it doesn't exist.
> if SECTIONP is non-null, return a pointer to the option's section.
> OPTION may be NULL. */
> @@ -794,7 +827,7 @@
> apr_pool_t *iteration_pool;
> int count;
>
> - find_option(cfg, section, NULL, &sec);
> + sec = find_section(cfg, section, pool);
> if (sec == NULL)
> return 0;
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-28 18:05:01 CEST