[PATCH] erofs-utils: sort shared xattr

[Huang Jianan]

在 2021/10/21 15:25, Gao Xiang 写道:
> Hi Jianan,
>
> On Thu, Oct 21, 2021 at 10:58:47AM +0800, Huang Jianan via Linux-erofs wrote:
>> Sort shared xattr before writing to disk to ensure the consistency
>> of reproducible builds.
> How about adding it as an option?

Can we consider turning on this by default abd add some test cases to 
ensure that xattr
functionality?  It seems that this part of the modification has no 
effect on the overall
function.

>> ---
>>   lib/xattr.c | 34 ++++++++++++++++++++++++++++++----
>>   1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/xattr.c b/lib/xattr.c
>> index 196133a..f17e57e 100644
>> --- a/lib/xattr.c
>> +++ b/lib/xattr.c
>> @@ -171,7 +171,7 @@ static struct xattr_item *parse_one_xattr(const char *path, const char *key,
>>   	/* allocate key-value buffer */
>>   	len[0] = keylen - prefixlen;
>>   
>> -	kvbuf = malloc(len[0] + len[1]);
>> +	kvbuf = malloc(len[0] + len[1] + 1);
>>   	if (!kvbuf)
>>   		return ERR_PTR(-ENOMEM);
>>   	memcpy(kvbuf, key + prefixlen, len[0]);
>> @@ -196,6 +196,7 @@ static struct xattr_item *parse_one_xattr(const char *path, const char *key,
>>   			len[1] = ret;
>>   		}
>>   	}
>> +	kvbuf[len[0] + len[1]] = '\0';
>>   	return get_xattritem(prefix, kvbuf, len);
>>   }
>>   
>> @@ -398,7 +399,7 @@ static int erofs_droid_xattr_set_caps(struct erofs_inode *inode)
>>   	len[0] = sizeof("capability") - 1;
>>   	len[1] = sizeof(caps);
>>   
>> -	kvbuf = malloc(len[0] + len[1]);
>> +	kvbuf = malloc(len[0] + len[1] + 1);
>>   	if (!kvbuf)
>>   		return -ENOMEM;
>>   
>> @@ -409,6 +410,7 @@ static int erofs_droid_xattr_set_caps(struct erofs_inode *inode)
>>   	caps.data[1].permitted = (u32) (capabilities >> 32);
>>   	caps.data[1].inheritable = 0;
>>   	memcpy(kvbuf + len[0], &caps, len[1]);
>> +	kvbuf[len[0] + len[1]] = '\0';
>>   
>>   	item = get_xattritem(EROFS_XATTR_INDEX_SECURITY, kvbuf, len);
>>   	if (IS_ERR(item))
>> @@ -562,13 +564,23 @@ static struct erofs_bhops erofs_write_shared_xattrs_bhops = {
>>   	.flush = erofs_bh_flush_write_shared_xattrs,
>>   };
>>   
>> +static int comp_xattr_item(const void *a, const void *b)
>> +{
>> +	const struct xattr_item *ia, *ib;
>> +
>> +	ia = (*((const struct inode_xattr_node **)a))->item;
>> +	ib = (*((const struct inode_xattr_node **)b))->item;
>> +
>> +	return strcmp(ia->kvbuf, ib->kvbuf);
> could we use strncmp (len[0] + len[1]) instead?

It seems that strncmp can't guarantee the order since ia and ib has 
different len. We
have ensure kvbuf[len[0] + len[1]] = '\0', Is there anything else to 
consider?

Thanks,
Jianan

> Thanks,
> Gao Xiang