trusted domains n+4 and related stuff

[tridge at samba.org]

Mimir,

Thanks for the patch!

A few comments:

- in ipstr_list_add you try to be too fancy. I suspect the chunking
  stuff is to try to second guess the malloc implementation and
  allocate in bigger lumps? Don't do that unless there is good
  profiling evidence to suggest that it is needed. It is too easy to
  get wrong and just complicates the code.

- in ipstr_list_add you do this:

	if (ipstr)
		safe_strcat(ipstr, ":", sizeof(ipstr));
	else
		return NULL;

  but ipstr is a fstring, so it can never be null. Only pointers can be
  NULL, and ipstr is an array (arrays can never be NULL in C).

A simpler varient of that function would be something like this
(untested code warning ...)

char* ipstr_list_add(char **ipstr_list, const struct in_addr *ip)
{
	char *new_str = NULL;

	if (*ipstr_list) {
		asprintf(&new_str, "%s:%s", *ipstr_list, inet_ntoa(*ip));
		free(*ipstr_list);
	} else {
		new_str = strdup(inet_ntoa(*ip));
	}
	
	*ipstr_list = new_str;
	return new_str;
}

and yes, I know its not terribly allocator efficient, but it has the
big advantage of being simple. I doubt allocator efficiency matters in
this function as the lists will typically be very short. Also note
that I removed the redundent ipstr_size argument. As the strings are
null terminated it isn't needed (unless you're playing allocator
efficiency games).

Similarly, ipstr_list_make() can be made much simpler.

- I think it might be better to use ',' instead of ':' for IP list
  separation. It doesn't matter now, but when we come to do IPv6 then
  it might matter, as iirc IPv6 uses ':' inside the string
  representation of addresses.

The parse function also tries to play allocation games. A simpler
function might be something like this:

int ipstr_list_parse(const char* ipstr_list, struct in_addr** ip_list)
{
	int count;
	for (ip_list=NULL, count=0; ipstr_list; count++) {
		struct in_addr a;

		if (inet_aton(ipstr_list, &a) == -1) break;

		*ip_list = Realloc(*ip_list, (count+1) * sizeof(struct in_addr));
		if (!ip_list) {
			return -1;
		}

		(*ip_list)[count] = a;

		ipstr_list = strchr(ipstr_list, ':');
		if (ipstr_list) ipstr_list++;
	}
	return count;
}

The rest of the patch looks good!

Cheers, Tridge

--
http://samba.org/~tridge/