[PATCH] erofs-utils: sort shared xattr

[Gao Xiang]

On Thu, Oct 21, 2021 at 05:28:25PM +0800, Huang Jianan wrote:
> 
> 
> 在 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.

Ok, that is fine with me as well.

> 
> > > ---
> > >   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?

you could min(xa, xb) and strncmp first. If strncmp returns 0, compare
length then.

Thanks,
Gao Xiang

> 
> Thanks,
> Jianan
> 
> > Thanks,
> > Gao Xiang