Divorce plugin rc from scap rc

Completely divorce plugin rc values from scap rc values.

Instead of being int32_t, plugin rc values are a ss_plugin_rc enum and
don't directly relate to SCAP_XXX values.

In a couple of locations in scap.c, plugin rcs do need to be mapped to
scap rcs to reflect opening sources, getting next event. Create a
function plugin_rc_to_scap_rc for that.

Also cleaned up a few other cases where ints were being used instead
of enums (plugin type, return value from next, etc.)

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>

userspace/libscap/plugin_info.h

@@ -60,20 +60,15 @@ typedef enum ss_plugin_field_type
 }ss_plugin_field_type;
 
 // Values to return from init() / open() / next() / next_batch() /
-// extract_fields(). Note that these values map exactly to the
-// corresponding SCAP_XXX values in scap.h, and should be kept in
-// sync.
-#define SS_PLUGIN_SUCCESS 0
-#define SS_PLUGIN_FAILURE 1
-#define SS_PLUGIN_TIMEOUT -1
-#define SS_PLUGIN_ILLEGAL_INPUT 3
-#define SS_PLUGIN_NOTFOUND 4
-#define SS_PLUGIN_INPUT_TOO_SMALL 5
-#define SS_PLUGIN_EOF 6
-#define SS_PLUGIN_UNEXPECTED_BLOCK 7
-#define SS_PLUGIN_VERSION_MISMATCH 8
-#define SS_PLUGIN_NOT_SUPPORTED 9
-
+// extract_fields().
+typedef enum ss_plugin_rc
+{
+	SS_PLUGIN_SUCCESS = 0,
+	SS_PLUGIN_FAILURE = 1,
+	SS_PLUGIN_TIMEOUT = -1,
+	SS_PLUGIN_EOF = 2,
+	SS_PLUGIN_NOT_SUPPORTED = 3,
+} ss_plugin_rc;
 
 // This struct represents an event returned by the plugin, and is used
 // below in next()/next_batch().
@@ -202,13 +197,13 @@ typedef struct
 	// Arguments:
 	// - config: a string with the plugin configuration. The format of the
 	//   string is chosen by the plugin itself.
-	// - rc: pointer to an integer that will contain the initialization result,
+	// - rc: pointer to a ss_plugin_rc that will contain the initialization result,
 	//   as a SS_PLUGIN_* value (e.g. SS_PLUGIN_SUCCESS=0, SS_PLUGIN_FAILURE=1)
 	// Return value: pointer to the plugin state that will be treated as opaque
 	//   by the engine and passed to the other plugin functions.
 	//   If rc is SS_PLUGIN_FAILURE, this function should return NULL.
 	//
-	ss_plugin_t* (*init)(char* config, int32_t* rc);
+	ss_plugin_t* (*init)(char* config, ss_plugin_rc* rc);
 	//
 	// Destroy the plugin and, if plugin state was allocated, free it.
 	// Required: yes
@@ -298,12 +293,12 @@ typedef struct
 	// - s: the plugin state returned by init()
 	// - params: the open parameters, as a string. The format is defined by the plugin
 	//   itsef
-	// - rc: pointer to an integer that will contain the open result, as a SS_PLUGIN_* value
-	//   (e.g. SS_PLUGIN_SUCCESS=0, SS_PLUGIN_FAILURE=1)
+	// - rc: pointer to a ss_plugin_rc that will contain the open result,
+	//   as a SS_PLUGIN_* value (e.g. SS_PLUGIN_SUCCESS=0, SS_PLUGIN_FAILURE=1)
 	// Return value: a pointer to the open context that will be passed to next(),
 	//   close(), event_to_string() and extract_fields.
 	//
-	ss_instance_t* (*open)(ss_plugin_t* s, char* params, int32_t* rc);
+	ss_instance_t* (*open)(ss_plugin_t* s, char* params, ss_plugin_rc* rc);
 	//
 	// Close a capture.
 	// Required: yes
@@ -328,7 +323,7 @@ typedef struct
 	// Return value: the status of the operation (e.g. SS_PLUGIN_SUCCESS=0, SS_PLUGIN_FAILURE=1,
 	//   SS_PLUGIN_TIMEOUT=-1)
 	//
-	int32_t (*next)(ss_plugin_t* s, ss_instance_t* h, ss_plugin_event **evt);
+	ss_plugin_rc (*next)(ss_plugin_t* s, ss_instance_t* h, ss_plugin_event **evt);
 	//
 	// Return the read progress.
 	// Required: no
@@ -365,9 +360,9 @@ typedef struct
 	// - fields: an array of ss_plugin_extract_field structs. Each element contains
 	//   a single field + optional arg, and the corresponding extracted value should
 	//   be in the same struct.
-	// Return value: An integer with values SS_PLUGIN_SUCCESS or SS_PLUGIN_FAILURE.
+	// Return value: An ss_plugin_rc with values SS_PLUGIN_SUCCESS or SS_PLUGIN_FAILURE.
 	//
-	int32_t (*extract_fields)(ss_plugin_t *s, const ss_plugin_event *evt, uint32_t num_fields, ss_plugin_extract_field *fields);
+	ss_plugin_rc (*extract_fields)(ss_plugin_t *s, const ss_plugin_event *evt, uint32_t num_fields, ss_plugin_extract_field *fields);
 	//
 	// This is an optional, internal, function used to speed up event capture by
 	// batching the calls to next().
@@ -380,7 +375,7 @@ typedef struct
 	//     owned by the plugin framework and will free them using plugin_free_mem().
 	// Required: no
 	//
-	int32_t (*next_batch)(ss_plugin_t* s, ss_instance_t* h, uint32_t *nevts, ss_plugin_event **evts);
+	ss_plugin_rc (*next_batch)(ss_plugin_t* s, ss_instance_t* h, uint32_t *nevts, ss_plugin_event **evts);
 
 	//
 	// The following members are PRIVATE for the engine and should not be touched.
@@ -499,7 +494,6 @@ typedef struct
 	//   array.
 	//
 	char* (*get_fields)();
-
 	//
 	// Extract one or more a filter field values from an event.
 	// Required: no
@@ -511,7 +505,7 @@ typedef struct
 	//   be in the same struct.
 	// Return value: An integer with values SS_PLUGIN_SUCCESS or SS_PLUGIN_FAILURE.
 	//
-	int32_t (*extract_fields)(ss_plugin_t *s, const ss_plugin_event *evt, uint32_t num_fields, ss_plugin_extract_field *fields);
+	ss_plugin_rc (*extract_fields)(ss_plugin_t *s, const ss_plugin_event *evt, uint32_t num_fields, ss_plugin_extract_field *fields);
 
 	//
 	// The following members are PRIVATE for the engine and should not be touched.

userspace/libscap/scap-int.h

@@ -20,6 +20,7 @@ limitations under the License.
 ////////////////////////////////////////////////////////////////////////////
 
 #include "settings.h"
+#include "plugin_info.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -195,7 +196,7 @@ struct scap
 	uint32_t m_input_plugin_batch_idx;
 
 	// The return value from the last call to batch_next().
-	int32_t m_input_plugin_last_batch_res;
+	ss_plugin_rc m_input_plugin_last_batch_res;
 };
 
 typedef enum ppm_dumper_type

userspace/libscap/scap.c

@@ -62,6 +62,34 @@ static const char *SYSDIG_BPF_PROBE_ENV = "SYSDIG_BPF_PROBE";
 //
 #define SCAP_PROBE_VERSION_SIZE 32
 
+static int32_t plugin_rc_to_scap_rc(ss_plugin_rc plugin_rc)
+{
+	switch(plugin_rc)
+	{
+	case SS_PLUGIN_SUCCESS:
+		return SCAP_SUCCESS;
+		break;
+	case SS_PLUGIN_FAILURE:
+		return SCAP_FAILURE;
+		break;
+	case SS_PLUGIN_TIMEOUT:
+		return SCAP_TIMEOUT;
+		break;
+	case SS_PLUGIN_EOF:
+		return SCAP_EOF;
+		break;
+	case SS_PLUGIN_NOT_SUPPORTED:
+		return SCAP_NOT_SUPPORTED;
+		break;
+	default:
+		ASSERT(false);
+		return SCAP_FAILURE;
+	}
+
+	ASSERT(false);
+	return SCAP_FAILURE;
+}
+
 const char* scap_getlasterr(scap_t* handle)
 {
 	return handle ? handle->m_lasterr : "null scap handle";
@@ -988,11 +1016,13 @@ scap_t* scap_open_plugin_int(char *error, int32_t *rc, source_plugin_info* input
 	// Set the rc to SCAP_FAILURE now, so in the unlikely event
 	// that a plugin doesn't not actually set a rc, that it gets
 	// treated as a failure.
-	*rc = SCAP_FAILURE;
+	ss_plugin_rc plugin_rc = SCAP_FAILURE;
 
 	handle->m_input_plugin->handle = handle->m_input_plugin->open(handle->m_input_plugin->state,
 		input_plugin_params,
-		rc);
+		&plugin_rc);
+
+	*rc = plugin_rc_to_scap_rc(plugin_rc);
 	handle->m_input_plugin_batch_nevts = 0;
 	handle->m_input_plugin_batch_evts = NULL;
 	handle->m_input_plugin_batch_idx = 0;
@@ -1717,7 +1747,7 @@ static int32_t scap_next_plugin(scap_t* handle, OUT scap_evt** pevent, OUT uint1
 		{
 			scap_free_plugin_batch_state(handle);
 
-			if(handle->m_input_plugin_last_batch_res != SCAP_SUCCESS)
+			if(handle->m_input_plugin_last_batch_res != SS_PLUGIN_SUCCESS)
 			{
 				if(handle->m_input_plugin_last_batch_res != SCAP_TIMEOUT && handle->m_input_plugin_last_batch_res != SCAP_EOF)
 				{
@@ -1770,17 +1800,17 @@ static int32_t scap_next_plugin(scap_t* handle, OUT scap_evt** pevent, OUT uint1
 	{
 		should_free_plugin_evt = true;
 
-		res = handle->m_input_plugin->next(handle->m_input_plugin->state,
-						   handle->m_input_plugin->handle, &plugin_evt);
-		if(res != SCAP_SUCCESS)
+		ss_plugin_rc plugin_res = handle->m_input_plugin->next(handle->m_input_plugin->state,
+								       handle->m_input_plugin->handle, &plugin_evt);
+		if(plugin_res != SS_PLUGIN_SUCCESS)
 		{
-			if(res != SCAP_TIMEOUT && res != SCAP_EOF)
+			if(plugin_res != SS_PLUGIN_TIMEOUT && res != SS_PLUGIN_EOF)
 			{
 				char *errstr = handle->m_input_plugin->get_last_error(handle->m_input_plugin->state);
 				snprintf(handle->m_lasterr, SCAP_LASTERR_SIZE, "%s", errstr);
 				handle->m_input_plugin->free_mem(errstr);
 			}
-			return res;
+			return plugin_rc_to_scap_rc(plugin_res);
 		}
 
 		res = SCAP_SUCCESS;

userspace/libsinsp/plugin.cpp

@@ -441,15 +441,15 @@ std::shared_ptr<sinsp_plugin> sinsp_plugin::create_plugin(string &filepath, char
 		return ret;
 	}
 
-	uint32_t (*get_type)();
+	ss_plugin_type (*get_type)();
 	*(void **) (&get_type) = getsym(handle, "plugin_get_type", errstr);
 	if(get_type == NULL)
 	{
 		errstr = string("Could not resolve plugin_get_type function");
 		return ret;
 	}
 
-	uint32_t plugin_type = get_type();
+	ss_plugin_type plugin_type = get_type();
 
 	sinsp_source_plugin *splugin;
 	sinsp_extractor_plugin *eplugin;
@@ -527,10 +527,10 @@ bool sinsp_plugin::init(char *config)
 		return false;
 	}
 
-	int32_t rc;
+	ss_plugin_rc rc;
 
 	ss_plugin_t *state = m_plugin_info.init(config, &rc);
-	if(rc != SCAP_SUCCESS)
+	if(rc != SS_PLUGIN_SUCCESS)
 	{
 		// Not calling get_last_error here because there was
 		// no valid ss_plugin_t struct returned from init.
@@ -615,11 +615,11 @@ bool sinsp_plugin::extract_field(ss_plugin_event &evt, sinsp_plugin::ext_field &
 	efield.arg = field.arg.c_str();
 	efield.ftype = field.ftype;
 
-	int32_t rc;
+	ss_plugin_rc rc;
 
 	rc = m_plugin_info.extract_fields(plugin_state(), &evt, num_fields, &efield);
 
-	if (rc != SCAP_SUCCESS)
+	if (rc != SS_PLUGIN_SUCCESS)
 	{
 		return false;
 	}
@@ -835,9 +835,9 @@ source_plugin_info *sinsp_source_plugin::plugin_info()
 	return &m_source_plugin_info;
 }
 
-bool sinsp_source_plugin::open(char *params, int32_t &rc)
+bool sinsp_source_plugin::open(char *params, ss_plugin_rc &rc)
 {
-	int32_t orc;
+	ss_plugin_rc orc;
 
 	if(!plugin_state())
 	{

userspace/libsinsp/plugin.h

@@ -136,15 +136,15 @@ class sinsp_plugin
 	typedef struct {
 		char* (*get_required_api_version)();
 		void (*free_mem)(void *ptr);
-		ss_plugin_t* (*init)(char* config, int32_t* rc);
+		ss_plugin_t* (*init)(char* config, ss_plugin_rc* rc);
 		void (*destroy)(ss_plugin_t* s);
 		char* (*get_last_error)(ss_plugin_t* s);
 		char* (*get_name)();
 		char* (*get_description)();
 		char* (*get_contact)();
 		char* (*get_version)();
 		char* (*get_fields)();
-		int32_t (*extract_fields)(ss_plugin_t *s, const ss_plugin_event *evt, uint32_t num_fields, ss_plugin_extract_field *fields);
+		ss_plugin_rc (*extract_fields)(ss_plugin_t *s, const ss_plugin_event *evt, uint32_t num_fields, ss_plugin_extract_field *fields);
 	} common_plugin_info;
 
 	std::string m_name;
@@ -179,7 +179,7 @@ class sinsp_source_plugin : public sinsp_plugin
 
 	// Note that embedding ss_instance_t in the object means that
 	// a plugin can only have one open active at a time.
-	bool open(char *params, int32_t &rc);
+	bool open(char *params, ss_plugin_rc &rc);
 	void close();
 	std::string get_progress(uint32_t &progress_pct);