sr: session: Make most functions return int.

This allows us to return error codes etc.

Add a little more error handling.

session.c

@@ -71,24 +71,26 @@ struct sr_session *sr_session_new(void)
  *
  * This frees up all memory used by the session.
  *
- * TODO: Should return int?
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
  */
-void sr_session_destroy(void)
+int sr_session_destroy(void)
 {
 	if (!session) {
 		sr_warn("session: %s: session was NULL", __func__);
-		return; /* TODO: SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	g_slist_free(session->devices);
+	session->devices = NULL;
 
 	/* TODO: Error checks needed? */
 
 	/* TODO: Loop over protocol decoders and free them. */
 
 	g_free(session);
-
 	session = NULL;
+
+	return SR_OK;
 }
 
 /**
@@ -97,17 +99,19 @@ void sr_session_destroy(void)
  * The session itself (i.e., the struct sr_session) is not free'd and still
  * exists after this function returns.
  *
- * TODO: Return int?
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
  */
-void sr_session_device_clear(void)
+int sr_session_device_clear(void)
 {
 	if (!session) {
 		sr_warn("session: %s: session was NULL", __func__);
-		return; /* TODO: SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	g_slist_free(session->devices);
 	session->devices = NULL;
+
+	return SR_OK;
 }
 
 /**
@@ -157,45 +161,48 @@ int sr_session_device_add(struct sr_device *device)
 /**
  * Clear all datafeed callbacks in the current session.
  *
- * TODO: Should return int?
+ * TODO.
  *
- * TODO
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
  */
-void sr_session_datafeed_callback_clear(void)
+int sr_session_datafeed_callback_clear(void)
 {
 	if (!session) {
 		sr_err("session: %s: session was NULL", __func__);
-		return; /* TODO: SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	g_slist_free(session->datafeed_callbacks);
 	session->datafeed_callbacks = NULL;
+
+	return SR_OK;
 }
 
 /**
  * Add a datafeed callback to the current session.
  *
- * TODO: Should return int?
+ * @param callback TODO.
- *
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
- * @param callback TODO
  */
-void sr_session_datafeed_callback_add(sr_datafeed_callback callback)
+int sr_session_datafeed_callback_add(sr_datafeed_callback callback)
 {
 	if (!session) {
 		sr_err("session: %s: session was NULL", __func__);
-		return; /* TODO: SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	/* TODO: Is 'callback' allowed to be NULL? */
 
 	session->datafeed_callbacks =
 	    g_slist_append(session->datafeed_callbacks, callback);
+
+	return SR_OK;
 }
 
 /**
  * TODO.
  */
-static void sr_session_run_poll(void)
+static int sr_session_run_poll(void)
 {
 	GPollFD *fds, my_gpollfd;
 	int ret, i;
@@ -208,6 +215,10 @@ static void sr_session_run_poll(void)
 		/* Construct g_poll()'s array. */
 		/* TODO: Check malloc return value. */
 		fds = malloc(sizeof(GPollFD) * num_sources);
+		if (!fds) {
+			sr_err("session: %s: fds malloc failed", __func__);
+			return SR_ERR_MALLOC;
+		}
 		for (i = 0; i < num_sources; i++) {
 #ifdef _WIN32
 			g_io_channel_win32_make_pollfd(&channels[0],
@@ -236,6 +247,8 @@ static void sr_session_run_poll(void)
 		}
 	}
 	free(fds);
+
+	return SR_OK;
 }
 
 /**
@@ -287,24 +300,23 @@ int sr_session_start(void)
 /**
  * Run the session.
  *
- * TODO: Should return int.
  * TODO: Various error checks etc.
  *
- * TODO.
+ * @return SR_OK upon success, SR_ERR_BUG upon errors.
  */
-void sr_session_run(void)
+int sr_session_run(void)
 {
 	if (!session) {
 		sr_err("session: %s: session was NULL; a session must be "
 		       "created first, before running it.", __func__);
-		return; /* TODO: SR_ERR or SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	if (!session->devices) {
 		/* TODO: Actually the case? */
 		sr_err("session: %s: session->devices was NULL; a session "
 		       "cannot be run without devices.", __func__);
-		return; /* TODO: SR_ERR or SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	sr_info("session: running");
@@ -320,27 +332,27 @@ void sr_session_run(void)
 		sr_session_run_poll();
 	}
 
-	/* TODO: return SR_OK; */
+	return SR_OK;
 }
 
 /**
  * Halt the current session.
  *
- * TODO: Return int.
- *
  * TODO.
+ *
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
  */
-void sr_session_halt(void)
+int sr_session_halt(void)
 {
 	if (!session) {
 		sr_err("session: %s: session was NULL", __func__);
-		return; /* TODO: SR_ERR; or SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	sr_info("session: halting");
 	session->running = FALSE;
 
-	/* TODO: return SR_OK; */
+	return SR_OK;
 }
 
 /**
@@ -348,51 +360,61 @@ void sr_session_halt(void)
  *
  * TODO: Difference to halt?
  *
- * TODO.
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
  */
-void sr_session_stop(void)
+int sr_session_stop(void)
 {
 	struct sr_device *device;
 	GSList *l;
 
 	if (!session) {
 		sr_err("session: %s: session was NULL", __func__);
-		return; /* TODO: SR_ERR or SR_ERR_BUG? */
+		return SR_ERR_BUG;
 	}
 
 	sr_info("session: stopping");
 	session->running = FALSE;
+
 	for (l = session->devices; l; l = l->next) {
 		device = l->data;
+		/* Check for device != NULL. */
 		if (device->plugin && device->plugin->stop_acquisition)
 			device->plugin->stop_acquisition(device->plugin_index, device);
 	}
 
-	/* TODO: return SR_OK; */
+	return SR_OK;
 }
 
 /**
  * TODO.
  *
- * TODO: Should return int?
  * TODO: Various error checks.
  *
  * @param packet TODO.
+ * @return SR_OK upon success, SR_ERR_ARG upon invalid arguments.
  */
-static void datafeed_dump(struct sr_datafeed_packet *packet)
+static int datafeed_dump(struct sr_datafeed_packet *packet)
 {
 	struct sr_datafeed_logic *logic;
 
+	if (!packet) {
+		sr_err("session: %s: packet was NULL", __func__);
+		return SR_ERR_ARG;
+	}
+
+	/* TODO: Validity checks for packet contents. */
+
 	switch (packet->type) {
 	case SR_DF_HEADER:
 		sr_dbg("bus: received SR_DF_HEADER");
 		break;
 	case SR_DF_TRIGGER:
 		sr_dbg("bus: received SR_DF_TRIGGER at %lu ms",
-				packet->timeoffset / 1000000);
+		       packet->timeoffset / 1000000);
 		break;
 	case SR_DF_LOGIC:
 		logic = packet->payload;
+		/* TODO: Check for logic != NULL. */
 		sr_dbg("bus: received SR_DF_LOGIC at %f ms duration %f ms, "
 		       "%" PRIu64 " bytes", packet->timeoffset / 1000000.0,
 		       packet->duration / 1000000.0, logic->length);
@@ -401,33 +423,39 @@ static void datafeed_dump(struct sr_datafeed_packet *packet)
 		sr_dbg("bus: received SR_DF_END");
 		break;
 	default:
-		/* TODO: sr_err() and abort? */
+		/* TODO: Abort? */
-		sr_dbg("bus: received unknown packet type %d", packet->type);
+		sr_err("bus: received unknown packet type %d", packet->type);
 		break;
 	}
+
+	return SR_OK;
 }
 
 /**
  * TODO.
  *
- * TODO: Should return int?
- *
  * @param device TODO.
  * @param packet TODO.
+ * @return SR_OK upon success, SR_ERR_ARG upon invalid arguments.
  */
-void sr_session_bus(struct sr_device *device, struct sr_datafeed_packet *packet)
+int sr_session_bus(struct sr_device *device, struct sr_datafeed_packet *packet)
 {
 	GSList *l;
 	sr_datafeed_callback cb;
 
 	if (!device) {
 		sr_err("session: %s: device was NULL", __func__);
-		return; /* TODO: SR_ERR_ARG */
+		return SR_ERR_ARG;
 	}
 
 	if (!device->plugin) {
 		sr_err("session: %s: device->plugin was NULL", __func__);
-		return; /* TODO: SR_ERR_ARG */
+		return SR_ERR_ARG;
+	}
+
+	if (!packet) {
+		sr_err("session: %s: packet was NULL", __func__);
+		return SR_ERR_ARG;
 	}
 
 	/*
@@ -441,13 +469,12 @@ void sr_session_bus(struct sr_device *device, struct sr_datafeed_packet *packet)
 		cb(device, packet);
 	}
 
-	/* TODO: return SR_OK; */
+	return SR_OK;
 }
 
 /**
  * TODO.
  *
- * TODO: Should return int?
  * TODO: Switch to g_try_malloc0() / g_free().
  * TODO: More error checks etc.
  *
@@ -456,15 +483,17 @@ void sr_session_bus(struct sr_device *device, struct sr_datafeed_packet *packet)
  * @param timeout TODO.
  * @param callback TODO.
  * @param user_data TODO.
+ * @return SR_OK upon success, SR_ERR_ARG upon invalid arguments, or
+ *         SR_ERR_MALLOC upon memory allocation errors.
  */
-void sr_session_source_add(int fd, int events, int timeout,
+int sr_session_source_add(int fd, int events, int timeout,
 	        sr_receive_data_callback callback, void *user_data)
 {
 	struct source *new_sources, *s;
 
 	if (!callback) {
 		sr_err("session: %s: callback was NULL", __func__);
-		return; /* TODO: SR_ERR_ARG */
+		return SR_ERR_ARG;
 	}
 
 	/* Note: user_data can be NULL, that's not a bug. */
@@ -472,7 +501,7 @@ void sr_session_source_add(int fd, int events, int timeout,
 	new_sources = calloc(1, sizeof(struct source) * (num_sources + 1));
 	if (!new_sources) {
 		sr_err("session: %s: new_sources malloc failed", __func__);
-		return; /* TODO: SR_ERR_MALLOC */
+		return SR_ERR_MALLOC;
 	}
 
 	if (sources) {
@@ -493,31 +522,36 @@ void sr_session_source_add(int fd, int events, int timeout,
 	    && (source_timeout == -1 || timeout < source_timeout))
 		source_timeout = timeout;
 
-	/* TODO: return SR_OK; */
+	return SR_OK;
 }
 
 /**
  * Remove the source belonging to the specified file descriptor.
  *
- * TODO: Should return int.
  * TODO: More error checks.
  * TODO: Switch to g_try_malloc0() / g_free().
  *
  * @param fd TODO.
+ * @return SR_OK upon success, SR_ERR_ARG upon invalid arguments, or
+ *         SR_ERR_MALLOC upon memory allocation errors, SR_ERR_BUG upon
+ *         internal errors.
  */
-void sr_session_source_remove(int fd)
+int sr_session_source_remove(int fd)
 {
 	struct source *new_sources;
 	int old, new;
 
-	/* TODO */
+	if (!sources) {
-	if (!sources)
+		sr_err("session: %s: sources was NULL", __func__);
-		return;
+		return SR_ERR_BUG; /* TODO: Other? */
+	}
+
+	/* TODO: Check if 'fd' valid. */
 
 	new_sources = calloc(1, sizeof(struct source) * num_sources);
 	if (!new_sources) {
 		sr_err("session: %s: new_sources malloc failed", __func__);
-		return; /* TODO: SR_ERR_MALLOC */
+		return SR_ERR_MALLOC;
 	}
 
 	for (old = 0, new = 0; old < num_sources; old++) {
@@ -534,4 +568,6 @@ void sr_session_source_remove(int fd)
 		/* Target fd was not found. */
 		free(new_sources);
 	}
+
+	return SR_OK;
 }

sigrok-proto.h

@@ -100,25 +100,24 @@ typedef void (*sr_datafeed_callback) (struct sr_device *device,
 /* Session setup */
 int sr_session_load(const char *filename);
 struct sr_session *sr_session_new(void);
-void sr_session_destroy(void);
+int sr_session_destroy(void);
-void sr_session_device_clear(void);
+int sr_session_device_clear(void);
 int sr_session_device_add(struct sr_device *device);
 
 /* Datafeed setup */
-void sr_session_datafeed_callback_clear(void);
+int sr_session_datafeed_callback_clear(void);
-void sr_session_datafeed_callback_add(sr_datafeed_callback callback);
+int sr_session_datafeed_callback_add(sr_datafeed_callback callback);
 
 /* Session control */
 int sr_session_start(void);
-void sr_session_run(void);
+int sr_session_run(void);
-void sr_session_halt(void);
+int sr_session_halt(void);
-void sr_session_stop(void);
+int sr_session_stop(void);
-void sr_session_bus(struct sr_device *device,
+int sr_session_bus(struct sr_device *device, struct sr_datafeed_packet *packet);
-		    struct sr_datafeed_packet *packet);
 int sr_session_save(const char *filename);
-void sr_session_source_add(int fd, int events, int timeout,
+int sr_session_source_add(int fd, int events, int timeout,
 	        sr_receive_data_callback callback, void *user_data);
-void sr_session_source_remove(int fd);
+int sr_session_source_remove(int fd);
 
 /*--- input/input.c ---------------------------------------------------------*/
 

sigrok.h.in

@@ -54,7 +54,8 @@ extern "C" {
 #define SR_ERR               -1 /* Generic/unspecified error */
 #define SR_ERR_MALLOC        -2 /* Malloc/calloc/realloc error */
 #define SR_ERR_ARG           -3 /* Function argument error */
-#define SR_ERR_SAMPLERATE    -4 /* Incorrect samplerate */
+#define SR_ERR_BUG           -4 /* Errors hinting at internal bugs */
+#define SR_ERR_SAMPLERATE    -5 /* Incorrect samplerate */
 
 #define SR_MAX_NUM_PROBES 	64 /* Limited by uint64_t. */
 #define SR_MAX_PROBENAME_LEN	32