On Sun, Aug 02, 2026 at 09:25:07PM +0800, Gao Xiang wrote:
> Hi Martin,
>
> On Wed, Jul 29, 2026 at 09:52:08PM +0200, Martin Pitt wrote:
> > listxattr(2) makes no promise about the order it reports: while e.g.
> > ext4 returns a reproducible order, tmpfs varies it from inode to inode,
> > so building the same tree twice can lay the same set of xattrs out
> > differently and yield images that differ byte for byte. This makes the
> > erofs images unreproducible.
> >
> > Insert into the inode's list ordered by attribute name instead, and move
> > inline attributes onto the on-stack list with list_add_tail() so the
> > emitted order matches. This is the same approach as the shared attribute
> > pool already does with comp_shared_xattritem().
> >
> > Signed-off-by: Martin Pitt <[email protected]>
>
> Thanks for the patch!
>
> I wonder if the following diff works too (but untested):
>
...
>
> Since I'd like to unify comp_shared_xattritem, if yes, could you resend
> a new version (or if some bug happens) as this so I could merge this.
>
Sorry... It should be
diff --git a/lib/xattr.c b/lib/xattr.c
index a9486e4..6a8775b 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -400,17 +400,44 @@ static struct erofs_xattritem
*erofs_get_selabel_xattr(struct erofs_sb_info *sbi
return NULL;
}
+static int erofs_comp_xattritem(const void *a, const void *b)
+{
+ const struct erofs_xattritem *ia, *ib;
+ unsigned int la, lb;
+ int ret;
+
+ ia = *((const struct erofs_xattritem **)a);
+ ib = *((const struct erofs_xattritem **)b);
+ la = EROFS_XATTR_KVSIZE(ia->len);
+ lb = EROFS_XATTR_KVSIZE(ib->len);
+
+ ret = memcmp(ia->kvbuf, ib->kvbuf, min(la, lb));
+ if (ret != 0)
+ return ret;
+ return cmpsgn(la, lb);
+}
+
static int erofs_inode_xattr_add(struct list_head *hlist,
struct erofs_xattritem *item)
{
- struct erofs_inode_xattr_node *node;
+ struct erofs_inode_xattr_node *node, *pos;
node = malloc(sizeof(*node));
if (!node)
return -ENOMEM;
init_list_head(&node->list);
node->item = item;
- list_add(&node->list, hlist);
+
+ /*
+ * Keep each inode's xattrs ordered by name. listxattr(2) makes no
+ * promise about the order it reports, and tmpfs varies it from inode
+ * to inode, so appending in listing order would emit the same set of
+ * xattrs differently from run to run and make images unreproducible.
+ */
+ list_for_each_entry(pos, hlist, list)
+ if (erofs_comp_xattritem(item, pos->item) < 0)
+ break;
+ list_add_tail(&node->list, &pos->list);
return 0;
}
@@ -848,24 +875,6 @@ static unsigned int erofs_cleanxattrs(struct
erofs_xattrmgr *xamgr,
return count;
}
-static int comp_shared_xattritem(const void *a, const void *b)
-{
- const struct erofs_xattritem *ia, *ib;
- unsigned int la, lb;
- int ret;
-
- ia = *((const struct erofs_xattritem **)a);
- ib = *((const struct erofs_xattritem **)b);
- la = EROFS_XATTR_KVSIZE(ia->len);
- lb = EROFS_XATTR_KVSIZE(ib->len);
-
- ret = memcmp(ia->kvbuf, ib->kvbuf, min(la, lb));
- if (ret != 0)
- return ret;
-
- return la > lb;
-}
-
int erofs_xattr_flush_name_prefixes(struct erofs_importer *im, bool plain)
{
const struct erofs_importer_params *params = im->params;
@@ -1015,7 +1024,7 @@ int erofs_load_shared_xattrs_from_path(struct
erofs_sb_info *sbi, const char *pa
}
DBG_BUGON(i != sharedxattr_count);
sorted_n[i] = NULL;
- qsort(sorted_n, sharedxattr_count, sizeof(n), comp_shared_xattritem);
+ qsort(sorted_n, sharedxattr_count, sizeof(n), erofs_comp_xattritem);
buf = calloc(1, shared_xattrs_size);
if (!buf) {
@@ -1096,10 +1105,10 @@ char *erofs_export_xattr_ibody(struct erofs_inode
*inode)
item = node->item;
list_del(&node->list);
- /* move inline xattrs to the onstack list */
+ /* move inline xattrs to the onstack list, order preserved */
if (item->shared_xattr_id < 0 ||
header->h_shared_count >= UCHAR_MAX) {
- list_add(&node->list, &ilst);
+ list_add_tail(&node->list, &ilst);
continue;
}
