Re: [PATCH v3] erofs-utils: lib: refactor extended attribute name prefixes

[Gao Xiang <xiang@kernel.org>]

On Wed, Sep 13, 2023 at 11:18:04AM +0800, Jingbo Xu wrote:
> Previously, the extended attribute name in xattr_item was actually the
> part with the matched prefix stripped, which makes it clumsy to strip
> or delete a specific attribute from one file.
> 
> To fix this, make the complete attribute name stored in xattr_item.
> One thing worth noting is that the attribute name in xattr_item has a
> trailing '\0' for ease of name comparison, while the trailing '\0' will
> get stripped when writing to on-disk erofs_xattr_entry.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>
> ---
> v3:
> - introduce EROFS_XATTR_KSIZE() and EROFS_XATTR_KVSIZE() macro
> - make the full long name prefix (instead of the infix in v2) stored in
>   ea_type_node; and thus compare with the full long name prefix in
>   get_xattritem() directly
> - convert the data type of in-memory prefix index and prefix_len from
>   u8/u16 to unsigned int
> ---

Applied with the following diff:

diff --git a/lib/xattr.c b/lib/xattr.c
index 8119013..bf63a81 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -78,16 +78,16 @@
 
 #define EA_HASHTABLE_BITS 16
 
-/* extra one byte for the trailing `\0` of attribute name */
+/* one extra byte for the trailing `\0` of attribute name */
 #define EROFS_XATTR_KSIZE(kvlen)	(kvlen[0] + 1)
 #define EROFS_XATTR_KVSIZE(kvlen)	(EROFS_XATTR_KSIZE(kvlen) + kvlen[1])
 
 /*
- * @base_index:	the index of matched predefined short prefix
- * @prefix:	the index of matched long prefix if any;
+ * @base_index:	the index of the matched predefined short prefix
+ * @prefix:	the index of the matched long prefix, if any;
  *		same as base_index otherwise
- * @prefix_len:	the length of matched long prefix if any;
- *		the length of matched predefined short prefix otherwise
+ * @prefix_len:	the length of the matched long prefix if any;
+ *		the length of the matched predefined short prefix otherwise
  */
 struct xattr_item {
 	struct xattr_item *next_shared_xattr;
@@ -185,6 +185,7 @@ static unsigned int put_xattritem(struct xattr_item *item)
 static struct xattr_item *get_xattritem(char *kvbuf, unsigned int len[2])
 {
 	struct xattr_item *item;
+	struct ea_type_node *tnode;
 	unsigned int hash[2], hkey;
 
 	hkey = xattr_item_hash(kvbuf, len, hash);
@@ -221,20 +222,15 @@ static struct xattr_item *get_xattritem(char *kvbuf, unsigned int len[2])
 	item->shared_xattr_id = -1;
 	item->prefix = item->base_index;
 
-	if (cfg.c_extra_ea_name_prefixes) {
-		struct ea_type_node *tnode;
-
-		list_for_each_entry(tnode, &ea_name_prefixes, list) {
-			if (item->base_index == tnode->base_index &&
-			    !strncmp(tnode->type.prefix, kvbuf,
-				     tnode->type.prefix_len)) {
-				item->prefix = tnode->index;
-				item->prefix_len = tnode->type.prefix_len;
-				break;
-			}
+	list_for_each_entry(tnode, &ea_name_prefixes, list) {
+		if (item->base_index == tnode->base_index &&
+		    !strncmp(tnode->type.prefix, kvbuf,
+			     tnode->type.prefix_len)) {
+			item->prefix = tnode->index;
+			item->prefix_len = tnode->type.prefix_len;
+			break;
 		}
 	}
-
 	hash_add(ea_hashtable, &item->node, hkey);
 	return item;
 }
@@ -267,7 +263,7 @@ static struct xattr_item *parse_one_xattr(const char *path, const char *key,
 	kvbuf = malloc(EROFS_XATTR_KVSIZE(len));
 	if (!kvbuf)
 		return ERR_PTR(-ENOMEM);
-	sprintf(kvbuf, "%s", key);
+	memcpy(kvbuf, key, EROFS_XATTR_KSIZE(len));
 	if (len[1]) {
 		/* copy value to buffer */
 #ifdef HAVE_LGETXATTR
@@ -492,7 +488,7 @@ int erofs_setxattr(struct erofs_inode *inode, char *key,
 	if (!kvbuf)
 		return -ENOMEM;
 
-	sprintf(kvbuf, "%s", key);
+	memcpy(kvbuf, key, EROFS_XATTR_KSIZE(len));
 	memcpy(kvbuf + EROFS_XATTR_KSIZE(len), value, size);
 
 	item = get_xattritem(kvbuf, len);