writer: Fix return value of lcfs_node_unset_xattr

libcomposefs/lcfs-writer.c

@@ -1660,24 +1660,26 @@ int lcfs_node_unset_xattr(struct lcfs_node_s *node, const char *name)
 {
 	ssize_t index = find_xattr(node, name);
 
-	if (index >= 0) {
-		struct lcfs_xattr_s *xattr = &node->xattrs[index];
-		size_t value_len = xattr->value_len;
-		free(xattr->key);
-		free(xattr->value);
-		if (index != (ssize_t)node->n_xattrs - 1)
-			node->xattrs[index] = node->xattrs[node->n_xattrs - 1];
-		node->n_xattrs--;
-		// Note 2*size - to account for worst case alignment
-		if (node->n_xattrs > 0)
-			node->xattr_size -= (2 * LCFS_INODE_XATTRMETA_SIZE - 1) +
-					    strlen(name) + value_len;
-		else
-			node->xattr_size = 0; // If last xattr, remove the overhead too
-		assert(node->xattr_size >= 0);
+	if (index < 0) {
+		errno = ENODATA;
+		return -1;
 	}
 
-	return -1;
+	struct lcfs_xattr_s *xattr = &node->xattrs[index];
+	size_t value_len = xattr->value_len;
+	free(xattr->key);
+	free(xattr->value);
+	if (index != (ssize_t)node->n_xattrs - 1)
+		node->xattrs[index] = node->xattrs[node->n_xattrs - 1];
+	node->n_xattrs--;
+	// Note 2*size - to account for worst case alignment
+	if (node->n_xattrs > 0)
+		node->xattr_size -= (2 * LCFS_INODE_XATTRMETA_SIZE - 1) +
+				    strlen(name) + value_len;
+	else
+		node->xattr_size = 0; // If last xattr, remove the overhead too
+	assert(node->xattr_size >= 0);
+	return 0;
 }
 
 /* Set an extended attribute; If from_external_input is true then we
@@ -1702,7 +1704,8 @@ int lcfs_node_set_xattr_internal(struct lcfs_node_s *node, const char *name,
 	}
 
 	// Remove any existing value
-	(void)lcfs_node_unset_xattr(node, name);
+	if (lcfs_node_unset_xattr(node, name) < 0 && errno != ENODATA)
+		return -1;
 
 	// Double the xattr metadata size, subtracting 1 to account for worst case alignment.
 	size_t entry_size = (2 * LCFS_INODE_XATTRMETA_SIZE) - 1 + namelen + value_len;

tests/test-lcfs.c

@@ -3,6 +3,7 @@
 
 #include "lcfs-writer.h"
 #include "lcfs-mount.h"
+#include <string.h>
 #include <assert.h>
 #include <unistd.h>
 #include <errno.h>
@@ -58,6 +59,49 @@ static void test_basic(void)
 	assert(r == 0);
 }
 
+static void test_xattr_addremove(void)
+{
+	cleanup_node struct lcfs_node_s *node = lcfs_node_new();
+	lcfs_node_set_mode(node, S_IFDIR | 0755);
+	cleanup_node struct lcfs_node_s *child = lcfs_node_new();
+	lcfs_node_set_mode(child, S_IFDIR | 0700);
+	int r = lcfs_node_unset_xattr(child, "user.foo");
+	int errsv = errno;
+	assert(r == -1);
+	assert(errsv == ENODATA);
+	r = lcfs_node_set_xattr(child, "user.foo", "bar", 3);
+	assert(r == 0);
+	r = lcfs_node_unset_xattr(child, "user.foo");
+	assert(r == 0);
+	r = lcfs_node_add_child(node, child, "somechild");
+	assert(r == 0);
+	child = NULL;
+}
+
+// Test that calling lcfs_node_set_xattr multiple times
+// with the same key has last-one-wins semantics.
+static void test_xattr_doubleadd(void)
+{
+	cleanup_node struct lcfs_node_s *node = lcfs_node_new();
+	lcfs_node_set_mode(node, S_IFDIR | 0755);
+	cleanup_node struct lcfs_node_s *child = lcfs_node_new();
+	lcfs_node_set_mode(child, S_IFDIR | 0700);
+	int r = lcfs_node_set_xattr(child, "user.foo", "bar", 3);
+	assert(r == 0);
+	// Should successfully silently overwrite.
+	r = lcfs_node_set_xattr(child, "user.foo", "baz", 3);
+	assert(r == 0);
+
+	size_t found_len;
+	const char *found_value = lcfs_node_get_xattr(child, "user.foo", &found_len);
+	assert(found_value);
+	assert(found_len == 3);
+	assert(memcmp(found_value, "baz", found_len) == 0);
+	r = lcfs_node_add_child(node, child, "somechild");
+	assert(r == 0);
+	child = NULL;
+}
+
 static void test_add_uninitialized_child(void)
 {
 	cleanup_node struct lcfs_node_s *node = lcfs_node_new();
@@ -99,4 +143,6 @@ int main(int argc, char **argv)
 	test_basic();
 	test_no_verity();
 	test_add_uninitialized_child();
+	test_xattr_addremove();
+	test_xattr_doubleadd();
 }