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

Re: svn commit: r33766 - branches/issue-2382/subversion/svnserve

From: Greg Stein <gstein_at_gmail.com>
Date: Sun, 19 Oct 2008 16:05:56 -0700

On Sun, Oct 19, 2008 at 3:18 PM, <stsp_at_tigris.org> wrote:
>...
> +++ branches/issue-2382/subversion/svnserve/listen.c Sun Oct 19 15:18:42 2008 (r33766)
> @@ -54,6 +54,16 @@ parse_addresses(apr_array_header_t **par
> apr_pool_t *iterpool = svn_pool_create(pool);
> *parsed_addresses = apr_array_make(pool, 1, sizeof(struct parsed_address *));
>
> + /* If no addresses were provided, listen on the default port
> + * on the unspecified address in all available address families. */
> + if (addresses->nelts == 0)
> + {
> + APR_ARRAY_PUSH(addresses, const char *) = APR_ANYADDR;
> +#if APR_HAVE_IPV6
> + APR_ARRAY_PUSH(addresses, const char *) = "::";
> +#endif
> + }

Hmm. This modifies the caller's array. I'm not sure you really want to
do that. IOW, best case you'd be marking "addresses" as const, which
would disallow something like this.

Looking at the code...

I'd recommend the following:

1) if addresses->nelts is 0, then push a parsed_address structure
directly into *parsed_addresses (could do this independent of other
changes; no need to push an address string, only to parse it)
2) early exit after doing (1)
3) defer creation of iterpool, in case (2) exits you early

Cheers,
-g

> +
> for (i = 0; i < addresses->nelts; i++)
> {
> const char *address;
> @@ -64,7 +74,7 @@ parse_addresses(apr_array_header_t **par
>
> svn_pool_clear(iterpool);
>
> - address = APR_ARRAY_IDX(addresses, i, const char*);
> + address = APR_ARRAY_IDX(addresses, i, const char *);
> status = apr_parse_addr_port(&host, &scope_id, &port, address, iterpool);
> if (status)
> return svn_error_wrap_apr(status,
> @@ -90,7 +100,7 @@ parse_addresses(apr_array_header_t **par
> }
>
> parsed_address = apr_palloc(pool, sizeof(struct parsed_address));
> - parsed_address->host = host;
> + parsed_address->host = apr_pstrdup(pool, host);
> parsed_address->port = port;
> APR_ARRAY_PUSH(*parsed_addresses, struct parsed_address *)
> = parsed_address;
> @@ -112,9 +122,6 @@ svn_error_t* init_listeners(apr_array_he
> apr_pool_t *parse_pool;
> apr_array_header_t *new_listeners;
>
> - /* If no addresses were specified, error out. */
> - SVN_ERR_ASSERT(addresses->nelts > 0);
> -
> *listeners = NULL;
> new_listeners = apr_array_make(pool, 1, sizeof(struct listener));
>
>
> Modified: branches/issue-2382/subversion/svnserve/main.c
> URL: http://svn.collab.net/viewvc/svn/branches/issue-2382/subversion/svnserve/main.c?pathrev=33766&r1=33765&r2=33766
> ==============================================================================
> --- branches/issue-2382/subversion/svnserve/main.c Sun Oct 19 14:38:14 2008 (r33765)
> +++ branches/issue-2382/subversion/svnserve/main.c Sun Oct 19 15:18:42 2008 (r33766)
> @@ -704,17 +704,6 @@ int main(int argc, const char *argv[])
> #endif
> }
>
> - if (addresses->nelts == 0)
> - {
> - /* No addresses to listen on were provided, so default to
> - * listen on the default port on the unspecified address
> - * in all available address families. */
> - APR_ARRAY_PUSH(addresses, const char *) = APR_ANYADDR;
> -#if APR_HAVE_IPV6
> - APR_ARRAY_PUSH(addresses, const char *) = "::";
> -#endif
> - }
> -
> err = init_listeners(&listeners, addresses, pool);
> if (err)
> return svn_cmdline_handle_exit_error(err, pool, "svnserve: ");
>
> Modified: branches/issue-2382/subversion/svnserve/server.h
> URL: http://svn.collab.net/viewvc/svn/branches/issue-2382/subversion/svnserve/server.h?pathrev=33766&r1=33765&r2=33766
> ==============================================================================
> --- branches/issue-2382/subversion/svnserve/server.h Sun Oct 19 14:38:14 2008 (r33765)
> +++ branches/issue-2382/subversion/svnserve/server.h Sun Oct 19 15:18:42 2008 (r33766)
> @@ -153,10 +153,15 @@ log_error(svn_error_t *err, apr_file_t *
> * Return an array of initialised listeners in *LISTENERS.
> * This should be passed to wait_for_client(), see below.
> *
> - * It is an error to call this function without specifying any address.
> - * If a string does not specify a port to listen on, the default port
> + * If an address does not specify a hostname, the unspecified address
> + * will be used (APR_ANYADDR for IPv4, "::" for IPv6).
> + *
> + * If an address does not specify a port to listen on, the default port
> * (SVN_RA_SVN_PORT) will be used.
> *
> + * If no addresses are provided, listen on the default port
> + * on the unspecified address in all available address families.
> + *
> * Do all allocations in POOL.
> */
> svn_error_t* init_listeners(apr_array_header_t **listeners,
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-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-10-20 01:06:30 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.