linux-kernel - Re: [PATCH V2 linux-next] fs: add BLOCKSIZE(inode)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20161209120327.1e75aaf0cc4c250d0a80f6b9@linux-foundation.org>
Date:   Fri, 9 Dec 2016 12:03:27 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Fabian Frederick <fabf@...net.be>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 linux-next] fs: add BLOCKSIZE(inode)

On Fri,  9 Dec 2016 20:38:33 +0100 Fabian Frederick <fabf@...net.be> wrote:

> Add (1 << inode->i_blkbits) macro for more readability
> #define BLOCKSIZE(inode) (1 << inode->i_blkbits)
> 
> This patch also fixes multiple checkpatch warnings:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 
> ...
>
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -543,6 +543,7 @@ is_uncached_acl(struct posix_acl *acl)
>  #define IOP_LOOKUP	0x0002
>  #define IOP_NOFOLLOW	0x0004
>  #define IOP_XATTR	0x0008
> +#define BLOCKSIZE(inode) (1 << inode->i_blkbits)
>  
>  /*
>   * Keep mostly read-only and often accessed (especially for

The name "BLOCKSIZE" is too generic.  Why should inodes consume this
identifier kernel-wide rather than, say, struct bio?

The return type is `int', yes?  I guess a 32-bit quantity is OK, but I
don't see why it should be signed.

So I'd suggest something along the lines of

static inline unsigned i_blocksize(const struct inode *inode)
{
	...
}