[DTrace-devel] [PATCH 1/2] Fix probe discovery loop

[eugene.loh at oracle.com]

From: Eugene Loh <eugene.loh at oracle.com>

In dt_provider_discover(), we loop over providers to discover new
probes.  For each existing provider whose implementation has a discovery
function, we call the function, thereby possibly adding new providers.

This has at least two problems:

  x The iterator is not designed to handle new table elements
    being added during the loop.

  x New providers that are added will presumably use the same
    implementation that discovered them.  So, new providers are
    simply adding redundant discovery functions.

While neither of these problems apparently causes any errors, the result
is a messy loop that leads potentially to unnecessary probe discovery.

Add dtp->dt_discs, a linked list of provider discovery functions.

Add functions to init and free this linked list.

Replace the iteration over a growing list of providers with an iteration
over the small, fixed list of provider discovery functions.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_impl.h     |  1 +
 libdtrace/dt_open.c     |  6 ++++++
 libdtrace/dt_provider.c | 45 +++++++++++++++++++++++++++++++++++------
 libdtrace/dt_provider.h |  2 ++
 4 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index 5282efbdb..ff730319b 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -333,6 +333,7 @@ struct dtrace_hdl {
 	struct dt_probe *dt_error; /* ERROR probe */
 
 	dt_htab_t *dt_provs;	/* hash table of dt_provider_t's */
+	dt_list_t dt_discs;	/* linked list of provider discovery functions */
 	const struct dt_provider *dt_prov_pid; /* PID provider */
 	int dt_proc_signal;	/* signal used to interrupt monitoring threads */
 	struct sigaction dt_proc_oact;
diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index a45908047..224f60cf7 100644
--- a/libdtrace/dt_open.c
+++ b/libdtrace/dt_open.c
@@ -1250,6 +1250,11 @@ dtrace_init(dtrace_hdl_t *dtp)
 			   dt_providers[i]->name);
 	}
 
+	/*
+	 * Initialize the list of provider discovery functions.
+	 */
+	dt_provider_discs_init(dtp);
+
 	return 0;
 }
 
@@ -1342,6 +1347,7 @@ dtrace_close(dtrace_hdl_t *dtp)
 	dt_probe_fini(dtp);
 
 	dt_htab_destroy(dtp->dt_provs);
+	dt_provider_discs_free(dtp);
 
 	for (i = 1; i < dtp->dt_cpp_argc; i++)
 		free(dtp->dt_cpp_argv[i]);
diff --git a/libdtrace/dt_provider.c b/libdtrace/dt_provider.c
index 079265b72..f7f76f9c2 100644
--- a/libdtrace/dt_provider.c
+++ b/libdtrace/dt_provider.c
@@ -231,18 +231,51 @@ dt_provider_xref(dtrace_hdl_t *dtp, dt_provider_t *pvp, id_t id)
 	return 0;
 }
 
-int
-dt_provider_discover(dtrace_hdl_t *dtp)
+typedef struct list_disc {
+	dt_list_t	list;
+	int		(*disc)(dtrace_hdl_t *dtp);
+} list_disc_t;
+
+void
+dt_provider_discs_init(dtrace_hdl_t *dtp)
 {
-	int		prid = dtp->dt_probe_id;
 	dt_htab_next_t	*it = NULL;
 	dt_provider_t	*pvp;
 
-	/* Discover new probes. */
 	while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL) {
-		if (pvp->impl->discover && pvp->impl->discover(dtp) < 0)
-			return -1;        /* errno is already set */
+		list_disc_t *disc;
+
+		if (pvp->impl->discover == NULL)
+			continue;
+		disc = dt_alloc(dtp, sizeof(list_disc_t));
+		if (disc == NULL)
+			return;
+		disc->disc = pvp->impl->discover;
+		dt_list_append(&dtp->dt_discs, disc);
 	}
+}
+
+void
+dt_provider_discs_free(dtrace_hdl_t *dtp)
+{
+	list_disc_t *disc;
+
+	while ((disc = dt_list_next(&dtp->dt_discs)) != NULL) {
+		dt_list_delete(&dtp->dt_discs, disc);
+		dt_free(dtp, disc);
+	}
+}
+
+int
+dt_provider_discover(dtrace_hdl_t *dtp)
+{
+	int		prid = dtp->dt_probe_id;
+	list_disc_t	*disc;
+
+	/* Discover new probes. */
+	for (disc = dt_list_next(&dtp->dt_discs); disc; disc = dt_list_next(disc))
+		if (disc->disc(dtp) < 0)
+			return -1;
 
 	/* Add them. */
 	for ( ; prid < dtp->dt_probe_id; prid++) {
diff --git a/libdtrace/dt_provider.h b/libdtrace/dt_provider.h
index dc1266711..0f8c13a00 100644
--- a/libdtrace/dt_provider.h
+++ b/libdtrace/dt_provider.h
@@ -120,6 +120,8 @@ extern dt_provider_t *dt_provider_create(dtrace_hdl_t *, const char *,
 					 const dtrace_pattr_t *, void *);
 extern int dt_provider_xref(dtrace_hdl_t *, dt_provider_t *, id_t);
 extern int dt_provider_discover(dtrace_hdl_t *dtp);
+extern void dt_provider_discs_init(dtrace_hdl_t *dtp);
+extern void dt_provider_discs_free(dtrace_hdl_t *dtp);
 
 #ifdef	__cplusplus
 }
-- 
2.47.3