Re: [PATCH v2 2/7] erofs: some marcos are much more readable as a function

From: Christoph Hellwig
Date: Fri Aug 30 2019 - 11:46:02 EST


On Thu, Aug 29, 2019 at 08:16:27PM -0700, Joe Perches wrote:
> > - sizeof(__u32) * ((__count) - 1); })
> > +static inline unsigned int erofs_xattr_ibody_size(__le16 d_icount)
> > +{
> > + unsigned int icount = le16_to_cpu(d_icount);
> > +
> > + if (!icount)
> > + return 0;
> > +
> > + return sizeof(struct erofs_xattr_ibody_header) +
> > + sizeof(__u32) * (icount - 1);
>
> Maybe use struct_size()?

Declaring a variable that is only used for struct_size is rather ugly.
But while we are nitpicking: you don't need to byteswap to check for 0,
so the local variable could be avoided.

Also what is that magic -1 for? Normally we use that for the
deprecated style where a variable size array is declared using
varname[1], but that doesn't seem to be the case for erofs.