[PATCH 2/2] alfred: Announce changed data only after it was stored

The event listeners were notified and the data type was marked as changed
before the buffer for the new data was allocated. When that allocation
fails then the dataset is left behind with a zero length and without
buffer. But "alfred -E" has already reported the update and the update
command will be started for a data type which was never stored (and which
lost its previous content).

Only notify the listeners and mark the type as changed after the new
data was copied into the dataset. And only free the old buffer when the new
was allocated.

Fixes: b106a7a36f98 ("alfred: notify event listener via unix socket")
Signed-off-by: Sven Eckelmann <[email protected]>
---
 recv.c      | 41 +++++++++++++++++++++++++----------------
 unix_sock.c | 43 ++++++++++++++++++++++++++++---------------
 2 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/recv.c b/recv.c
index 76bfb30..929d086 100644
--- a/recv.c
+++ b/recv.c
@@ -32,8 +32,10 @@ static int finish_alfred_push_data(struct globals *globals,
 	struct dataset *dataset;
 	bool new_entry_created;
 	unsigned int data_len;
+	unsigned char *buf;
 	unsigned int len;
 	uint8_t *pos;
+	bool changed;
 
 	/* test already done in process_alfred_push_data */
 	len = ntohs(push->header.length);
@@ -62,10 +64,6 @@ static int finish_alfred_push_data(struct globals *globals,
 			dataset->data_source = SOURCE_SYNCED;
 
 			memcpy(&dataset->data, data, sizeof(*data));
-			if (hash_add(globals->data_hash, dataset)) {
-				free(dataset);
-				goto err;
-			}
 			new_entry_created = true;
 		}
 		/* don't overwrite our own data */
@@ -75,13 +73,14 @@ static int finish_alfred_push_data(struct globals *globals,
 		clock_gettime(CLOCK_MONOTONIC, &dataset->last_seen);
 
 		/* check that data was changed */
-		if (new_entry_created ||
-		    dataset->data.header.length != data_len ||
-		    memcmp(dataset->buf, data->data, data_len) != 0) {
-			changed_data_type(globals, data->header.type);
-			unix_sock_event_notify(globals, data->header.type,
-					       data->source);
-		}
+		changed = new_entry_created ||
+			  dataset->data.header.length != data_len ||
+			  !dataset->buf ||
+			  memcmp(dataset->buf, data->data, data_len) != 0;
+
+		buf = malloc(data_len);
+		if (!buf)
+			goto err;
 
 		/* free old buffer */
 		if (dataset->buf) {
@@ -89,16 +88,26 @@ static int finish_alfred_push_data(struct globals *globals,
 			dataset->data.header.length = 0;
 		}
 
-		dataset->buf = malloc(data_len);
-
-		/* that's not good */
-		if (!dataset->buf)
-			goto err;
+		if (new_entry_created) {
+			if (hash_add(globals->data_hash, dataset)) {
+				free(buf);
+				free(dataset);
+				goto err;
+			}
+		}
 
+		dataset->buf = buf;
 		dataset->data.header.length = data_len;
 		dataset->data.header.version = data->header.version;
 		memcpy(dataset->buf, data->data, data_len);
 
+		/* only announce the new data once it is actually stored */
+		if (changed) {
+			changed_data_type(globals, data->header.type);
+			unix_sock_event_notify(globals, data->header.type,
+					       data->source);
+		}
+
 		/* if the sender is also the the source of the dataset, we
 		 * got a first hand dataset. */
 		if (memcmp(&mac, data->source, ETH_ALEN) == 0)
diff --git a/unix_sock.c b/unix_sock.c
index e6b44da..28752ec 100644
--- a/unix_sock.c
+++ b/unix_sock.c
@@ -106,8 +106,10 @@ static int unix_sock_add_data(struct globals *globals,
 	struct interface *interface;
 	struct alfred_data *data;
 	struct dataset *dataset;
+	unsigned char *buf;
 	int data_len;
 	int ret = -1;
+	bool changed;
 	int len;
 
 	len = ntohs(push->header.length);
@@ -166,34 +168,45 @@ static int unix_sock_add_data(struct globals *globals,
 		dataset->buf = NULL;
 
 		memcpy(&dataset->data, data, sizeof(*data));
-		if (hash_add(globals->data_hash, dataset)) {
-			free(dataset);
-			goto err;
-		}
 		new_entry_created = true;
 	}
 	dataset->data_source = SOURCE_LOCAL;
 	clock_gettime(CLOCK_MONOTONIC, &dataset->last_seen);
 
 	/* check that data was changed */
-	if (new_entry_created ||
-	    dataset->data.header.length != data_len ||
-	    memcmp(dataset->buf, data->data, data_len) != 0)
-		unix_sock_event_notify(globals, data->header.type,
-				       data->source);
+	changed = new_entry_created ||
+		  dataset->data.header.length != data_len ||
+		  !dataset->buf ||
+		  memcmp(dataset->buf, data->data, data_len) != 0;
 
-	/* free old buffer */
-	free(dataset->buf);
-
-	dataset->buf = malloc(data_len);
-	/* that's not good */
-	if (!dataset->buf)
+	buf = malloc(data_len);
+	if (!buf)
 		goto err;
 
+	/* free old buffer */
+	if (dataset->buf) {
+		free(dataset->buf);
+		dataset->data.header.length = 0;
+	}
+
+	if (new_entry_created) {
+		if (hash_add(globals->data_hash, dataset)) {
+			free(buf);
+			free(dataset);
+			goto err;
+		}
+	}
+
+	dataset->buf = buf;
 	dataset->data.header.length = data_len;
 	dataset->data.header.version = data->header.version;
 	memcpy(dataset->buf, data->data, data_len);
 
+	/* only announce the new data once it is actually stored */
+	if (changed)
+		unix_sock_event_notify(globals, data->header.type,
+				       data->source);
+
 	ret = 0;
 err:
 	close(client_sock);

-- 
2.47.3