Re: [PATCH 2/4] Refactoring: connect.c: move duplicated code to get_host_and_port

[Johannes Sixt <j6t@kdbg.org>]

Michael Lukashov schrieb:
> +static void get_host_and_port(char **host, const char **port, int set_port_none)

Minor nit: The last parameter, set_port_none, is a rather prominent sign 
that this function mixes policy and functionality. And indeed, this 
implementation:

> +	if (colon) {
> +		*colon = 0;
> +		*port = colon + 1;
> +		if (set_port_none && !**port)
> +			*port = "<none>";
> +	}

proves it. The _functionality_ is to find host and port from a string. The 
_policy_ is to set the port to "<none>" if it would otherwise be empty. 
The callers take care of the _policy_, this function should only care 
about _functionality_. There's only one call site that wants "<none>"; 
don't move this detail into this function.

Other than that: nice catch.

-- Hannes