libvmon: ensure dtor invocations only return dtor codes

libvmon's internal api for samplers is extremely ad-hoc and
implicit.

This was done at the time to keep the source compact and have the
sampler ctor/dtor branches commingled immediately adjacent to
eachother... the thinking being this would help keep them in sync
as the code evolved.  The ctor branches generally open fds and
allocate resources, with the dtor intended to undo those things.
With them kept more or less in the same page of code, it /should/
be obvious when one changes the other must as well.

In the long-term it probably makes sense to just explode this api
to something more formal and step back from those assumptions.

In lieu of doing such a refactor, let's improve the situation by
asserting the return codes at least stay within the expected
range.  i.e. let's abort when a sample_changed/unchanged/error
return code comes from an dtor invocation, since this implies a
program error where someone isn't handling the implicit dtor
branches properly, instead falling through sampling paths.

1 files changed, 12 insertions, 1 deletions

diff --git a/src/libvmon/vmon.c b/src/libvmon/vmon.c
index 8e8067f..9477531 100644
--- a/src/libvmon/vmon.c
+++ b/src/libvmon/vmon.c
@@ -20,6 +20,7 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <assert.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <stdio.h>
@@ -1290,8 +1291,18 @@ void vmon_proc_unmonitor(vmon_t *vmon, vmon_proc_t *proc, void (*sample_cb)(vmon
 
 	for (i = 0; i < sizeof(vmon->proc_funcs) / sizeof(vmon->proc_funcs[VMON_STORE_PROC_STAT]); i++) {
 		if (proc->stores[i] != NULL) {	/* any non-NULL stores must have a function installed and must have been sampled, invoke the dtor branch */
-			if (vmon->proc_funcs[i](vmon, NULL, &proc->stores[i]) == DTOR_FREE)
+			sample_ret_t	r;
+
+			r = vmon->proc_funcs[i](vmon, NULL, &proc->stores[i]);
+			switch (r) {
+			case DTOR_FREE:
 				try_free((void **)&proc->stores[i]);
+				break;
+			case DTOR_NOFREE:
+				break;
+			default:
+				assert(0);
+			}
 		}
 	}