Re: [PATCH] XATTR support on JFFS2 (version. 5)

Artem B. Bityutskiy
2006-05-10T10:04:39Z

On Sat, 2006-05-06 at 15:51 +0900, KaiGai Kohei wrote:
> This attached patches provide xattr support including POSIX-ACL and
> SELinux support on JFFS2 (version.5).
> We can apply those patches to the latest git-tree.
> - xattr_on_jffs2.kernel.version-5.patch can be applied to mtd-2.6.git.
> - xattr_on_jffs2.utils.version-5.patch can be applied to mtd-utils.git.

The patch in general looks nice. I have got no real
suggestions/objections for now, slowly trying to read it and delve into
(the patch is quite large, my time is very limited).

Few suggestions for now (again cosmetic):

1. GC-related functions and summary-related functions should be moved to
gc.c and summary.c
2. JFFS2 uses namings we all are used to. E.g., c - JFFS2 superblock
structure, ref - struct jffs2_node_ref. I've noticed you use 'ref' as
jffs2_xattr_ref pointers which is just a bit confusing. I would suggest
you calling them 'xref' - this would improve readability a bit.

Example of confusing code:

> + down_write(&c->xattr_sem);
> + list_for_each_entry_safe(ref, _ref, &ic->ilist, ilist)
> + delete_xattr_ref(c, ref);
> + up_write(&c->xattr_sem);

This would be more readable for those aware with "older" JFFS2
internals:

> + down_write(&c->xattr_sem);
> + list_for_each_entry_safe(xref, _xref, &ic->ilist, ilist)
> + delete_xattr_ref(c, xref);
> + up_write(&c->xattr_sem);

It's minor, but still.

Will you object if I just do some changes I consider useful in xattr
GIT? Or should I first mail you?

--
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.