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

Artem B. Bityutskiy
2006-05-07T13:12:53Z

Hi, few refinements,

On Sun, 2006-05-07 at 16:46 +0400, Artem B. Bityutskiy wrote:
> > +#include <linux/xattr.h>
> You're using 'struct list_head' in your 'xattr.h' file, wouldn't it be a
> good tone to add #include <linux/lists.h> then?
>
> > +struct jffs2_xattr_datum
> > +{
> > + void *always_null;
> > + u8 class;
> > + u8 flags;
> > + u16 xprefix; /* see JFFS2_XATTR_PREFIX_* */
> > +
> > + struct jffs2_raw_node_ref *node;
> > + struct list_head xindex; /* chained from c->xattrindex[n] */
> > + uint32_t refcnt; /* # of xattr_ref refers this */
> > + uint32_t xid;
> > + uint32_t version;
> > +
> > + uint32_t data_crc;
> > + uint32_t hashkey;
> > + char *xname; /* XATTR name without prefix */
> > + uint32_t name_len; /* length of xname */
> > + char *xvalue; /* XATTR value */
> > + uint32_t value_len; /* length of xvalue */
> > +};
> Would be cuter to use Linux-style comments.
I mean, this is not a requirement, this is just my opinion. But David is
the maintainer, and he does not agree here, so please, ignore this.

> > +struct jffs2_inode_cache; /* forward refence */
> A classic example of a senseless comment :-)
Err, sorry, I meant, this comment is not really useful and might be
removed.

> > +/*---- Any inline initialize functions ----*/
> > +#define init_xattr_inode_cache(x) INIT_LIST_HEAD(&((x)->ilist))
> Wierd comment.
How about to just remove this comment?

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