Make WINBINDD_LIST_GROUPS handler asynchronous.

Previously WINBINDD_LIST_GROUPS requests (ex: wbinfo -g) were handled by the
winbindd parent process in a sequential fashion.  This patch, delegates the work
to the winbindd children so that the request is handled much faster in large
domain topologies, and doesn't block the parent from receiving new requests.

The core group enumeration and conversion that was handled in
winbindd_list_groups() has been moved into winbindd_dual_list_groups() to be
done by the child.

The parent winbindd_list_groups() simply calls each of the children
asynchronously.

listgroups_recv() aggregates the final group list that will be returned to the
client and tracks how many of the children have returned their lists.

The domain name of the child is passed back through the callbacks to be used in
debugging messages.

There are also several fixes to typos in various comments.
(This used to be commit 037b9689d9042a398cb91e4628a82fcdfa913c21)

1 files changed, 78 insertions, 58 deletions

diff --git a/source3/winbindd/winbindd_group.c b/source3/winbindd/winbindd_group.c
index 63fde9f495..dd2fc6f6b5 100644
--- a/source3/winbindd/winbindd_group.c
+++ b/source3/winbindd/winbindd_group.c
@@ -971,10 +971,9 @@ void winbindd_endgrent(struct winbindd_cli_state *state)
 
 /* Get the list of domain groups and domain aliases for a domain.  We fill in
    the sam_entries and num_sam_entries fields with domain group information.  
-   The dispinfo_ndx field is incremented to the index of the next group to 
-   fetch. Return True if some groups were returned, False otherwise. */
+   Return True if some groups were returned, False otherwise. */
 
-static bool get_sam_group_entries(struct getent_state *ent)
+bool get_sam_group_entries(struct getent_state *ent)
 {
 	NTSTATUS status;
 	uint32 num_entries;
@@ -1340,15 +1339,23 @@ void winbindd_getgrent(struct winbindd_cli_state *state)
 		request_error(state);
 }
 
-/* List domain groups without mapping to unix ids */
+struct listgroups_state {
+	TALLOC_CTX *mem_ctx;
+	struct winbindd_cli_state *cli_state;
+	unsigned int domain_count;
+	char *extra_data;
+	unsigned int extra_data_len;
+};
 
+static void listgroups_recv(void *private_data, bool success, fstring dom_name,
+	                    char *extra_data);
+
+/* List domain groups without mapping to unix ids */
 void winbindd_list_groups(struct winbindd_cli_state *state)
 {
-	uint32 total_entries = 0;
 	struct winbindd_domain *domain;
 	const char *which_domain;
-	char *extra_data = NULL;
-	unsigned int extra_data_len = 0, i;
+	struct listgroups_state *groups_state;
 
 	DEBUG(3, ("[%5lu]: list groups\n", (unsigned long)state->pid));
 
@@ -1356,74 +1363,87 @@ void winbindd_list_groups(struct winbindd_cli_state *state)
 	state->request.domain_name[sizeof(state->request.domain_name)-1]='\0';	
 	which_domain = state->request.domain_name;
 	
-	/* Enumerate over trusted domains */
+	/* Initialize listgroups_state */
+	groups_state = TALLOC_P(state->mem_ctx, struct listgroups_state);
+	if (groups_state == NULL) {
+		DEBUG(0, ("talloc failed\n"));
+		request_error(state);
+		return;
+	}
 
-	for (domain = domain_list(); domain; domain = domain->next) {
-		struct getent_state groups;
+	groups_state->mem_ctx = state->mem_ctx;
+	groups_state->cli_state = state;
+	groups_state->domain_count = 0;
+	groups_state->extra_data = NULL;
+	groups_state->extra_data_len = 0;
 
+	/* Must count the full list of expected domains before we request data
+	 * from any of them.  Otherwise it's possible for a connection to the
+	 * first domain to fail, call listgroups_recv(), and return to the
+	 * client without checking any other domains. */
+	for (domain = domain_list(); domain; domain = domain->next) {
 		/* if we have a domain name restricting the request and this
 		   one in the list doesn't match, then just bypass the remainder
 		   of the loop */
-		   
 		if ( *which_domain && !strequal(which_domain, domain->name) )
 			continue;
-			
-		ZERO_STRUCT(groups);
 
-		/* Get list of sam groups */
-		
-		fstrcpy(groups.domain_name, domain->name);
-
-		get_sam_group_entries(&groups);
-			
-		if (groups.num_sam_entries == 0) {
-			/* this domain is empty or in an error state */
-			continue;
-		}
+		groups_state->domain_count++;
+	}
 
-		/* keep track the of the total number of groups seen so 
-		   far over all domains */
-		total_entries += groups.num_sam_entries;
-		
-		/* Allocate some memory for extra data.  Note that we limit
-		   account names to sizeof(fstring) = 128 characters.  */		
-		extra_data = (char *)SMB_REALLOC(
-			extra_data, sizeof(fstring) * total_entries);
- 
-		if (!extra_data) {
-			DEBUG(0,("failed to enlarge buffer!\n"));
-			request_error(state);
-			return;
-		}
+	/* Make sure we're enumerating at least one domain */
+	if (!groups_state->domain_count) {
+		request_ok(state);
+		return;
+	}
 
-		/* Pack group list into extra data fields */
-		for (i = 0; i < groups.num_sam_entries; i++) {
-			char *group_name = ((struct acct_info *)
-					    groups.sam_entries)[i].acct_name; 
-			fstring name;
-
-			fill_domain_username(name, domain->name, group_name, True);
-			/* Append to extra data */			
-			memcpy(&extra_data[extra_data_len], name, 
-                               strlen(name));
-			extra_data_len += strlen(name);
-			extra_data[extra_data_len++] = ',';
-		}
+	/* Enumerate list of trusted domains and request group list from each */
+	for (domain = domain_list(); domain; domain = domain->next) {
+		if ( *which_domain && !strequal(which_domain, domain->name) )
+			continue;
 
-		SAFE_FREE(groups.sam_entries);
+		winbindd_listgroups_async(state->mem_ctx, domain,
+			                  listgroups_recv, groups_state);
 	}
+}
+
+static void listgroups_recv(void *private_data, bool success, fstring dom_name,
+	                    char *extra_data)
+{
+	/* extra_data comes to us as a '\0' terminated string of comma
+	   separated groups */
+	struct listgroups_state *state = private_data;
 
-	/* Assign extra_data fields in response structure */
+	/* Append groups from one domain onto the whole list */
 	if (extra_data) {
-		extra_data[extra_data_len - 1] = '\0';
-		state->response.extra_data.data = extra_data;
-		state->response.length += extra_data_len;
+		DEBUG(5, ("listgroups_recv: %s returned groups.\n", dom_name));
+		if (!state->extra_data)
+			state->extra_data = talloc_asprintf(state->mem_ctx,
+							    "%s", extra_data);
+		else
+			state->extra_data = talloc_asprintf_append_buffer(
+							    state->extra_data,
+							    ",%s", extra_data);
+		/* Add one for the '\0' and each additional ',' */
+		state->extra_data_len += strlen(extra_data) + 1;
+	}
+	else {
+		DEBUG(5, ("listgroups_recv: %s returned no groups.\n",
+			dom_name));
 	}
 
-	/* No domains may have responded but that's still OK so don't
-	   return an error. */
+	if (--state->domain_count)
+		/* Still waiting for some child domains to return */
+		return;
 
-	request_ok(state);
+	/* Return list of all groups to the client */
+	if (state->extra_data) {
+		state->cli_state->response.extra_data.data =
+			SMB_STRDUP(state->extra_data);
+		state->cli_state->response.length += state->extra_data_len;
+	}
+
+	request_ok(state->cli_state);
 }
 
 /* Get user supplementary groups.  This is much quicker than trying to