Re: [PATCH v5 01/10] fs/ntfs3: Add headers and misc files

From: Joe Perches
Date: Fri Sep 11 2020 - 22:28:35 EST


On Fri, 2020-09-11 at 17:10 +0300, Konstantin Komarov wrote:
> This adds headers and misc files

The code may be ok, but its cosmetics are poor.

> diff --git a/fs/ntfs3/debug.h b/fs/ntfs3/debug.h
[]
> +#define QuadAlign(n) (((n) + 7u) & (~7u))
> +#define IsQuadAligned(n) (!((size_t)(n)&7u))
> +#define Quad2Align(n) (((n) + 15(&u) & (~15u))
> +#define IsQuad2Aligned(n) (!((size_t)(n)&15u))
> +#define Quad4Align(n) (((n) + 31u) & (~31u))
> +#define IsSizeTAligned(n) (!((size_t)(n) & (sizeof(size_t) - 1)))
> +#define DwordAlign(n) (((n) + 3u) & (~3u))
> +#define IsDwordAligned(n) (!((size_t)(n)&3u))
> +#define WordAlign(n) (((n) + 1u) & (~1u))
> +#define IsWordAligned(n) (!((size_t)(n)&1u))

All of these could use column alignment.
IsSizeTAligned could is the kernel's IS_ALIGNED

#define IsQuadAligned(n) (!((size_t)(n) & 7u))
#define QuadAlign(n) (((n) + 7u) & (~7u))
#define IsQuadAligned(n) (!((size_t)(n) & 7u))
#define Quad2Align(n) (((n) + 15u) & (~15u))
#define IsQuad2Aligned(n) (!((size_t)(n) & 15u))

Though all of these could also use two macros with
the same form. Something like:

#define NTFS3_ALIGN(n, at) (((n) + ((at) - 1)) & (~((at) - 1)))
#define NTFS3_IS_ALIGNED(n, at) (!((size_t)(n) & ((at) - 1)))

So all of these could be ordered by size and use actual size

#define WordAlign(n) NTFS3_ALIGN(n, 2)
#define IsWordAligned(n) NTFS3_IS_ALIGNED(n, 2)
#define DwordAlign(n) NTFS3_ALIGN(n, 4)
#define IsDwordAligned(n)
NTFS3_IS_ALIGNED(n, 4)
#define QuadAlign(n) NTFS3_ALIGN(n, 8)
#define IsQuadAligned(n) NTFS3_IS_ALIGNED(n, 8)
#define
Quad2Align(n) NTFS3_ALIGN(n, 16)
#define IsQuad2Aligned(n) NTFS3_IS_ALIGNED(n, 16)

#define IsSizeTAligned(n) NTFS3_IS_ALIGNED(n, sizeof(size_t))


> +#ifdef CONFIG_PRINTK
> +__printf(2, 3) void ntfs_printk(const struct super_block *sb, const char *fmt,
> + ...);

Better would be

__printf(2, 3)
void ntfs_printk(const struct super_block *sb, const char *fmt, ...);

etc...

There's a lot of code that could be made more readable for a human.