writer: Fix return value of lcfs_node_unset_xattr

Since the first creation of this code in
5ac1f5c "lib: Update xattr APIs" this function
has always returned an error code - but nothing
checked it, so it didn't matter.

Now also, currently this function cannot fail
but let's give ourselves some flexibility here;
perhaps we want to e.g. invoke `realloc`
and in that case we'd need to handle OOM.

I just noticed this while working on commit
02077e8
to reject empty xattr names.

Change this to return success, and also check its
return value in the set path.

Signed-off-by: Colin Walters <walters@verbum.org>

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

@@ -58,6 +58,25 @@ 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;
+}
+
 static void test_add_uninitialized_child(void)
 {
 	cleanup_node struct lcfs_node_s *node = lcfs_node_new();
@@ -99,4 +118,5 @@ int main(int argc, char **argv)
 	test_basic();
 	test_no_verity();
 	test_add_uninitialized_child();
+	test_xattr_addremove();
 }