| Message ID | 20201026215236.3894200-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tomoyo: fix clang pointer arithmetic warning | expand |
Thank you for a patch. I have two questions. On 2020/10/27 6:52, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > clang warns about additions on NULL pointers being undefined in C: > > security/tomoyo/securityfs_if.c:226:59: warning: arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension [-Wnull-pointer-arithmetic] > securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, > > Change the code to instead use a cast through uintptr_t to avoid > the warning. > > - securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, > + securityfs_create_file(name, mode, parent, (u8 *)(uintptr_t)key, > &tomoyo_operations); (1) Does clang warn if "(void *)key" is used instead of "(u8 *)(uintptr_t)key" ? (2) tomoyo_open() has const int key = ((u8 *) file_inode(file)->i_private) - ((u8 *) NULL); which decodes the "u8 key" passed to tomoyo_create_entry(). For symmetry, I'd like to remove NULL from tomoyo_open() as well. Does clang warn if const int key = (u8) (file_inode(file)->i_private); is used?
On Wed, Oct 28, 2020 at 2:22 PM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > Thank you for a patch. I have two questions. > > On 2020/10/27 6:52, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > clang warns about additions on NULL pointers being undefined in C: > > > > security/tomoyo/securityfs_if.c:226:59: warning: arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension [-Wnull-pointer-arithmetic] > > securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, > > > > Change the code to instead use a cast through uintptr_t to avoid > > the warning. > > > > > - securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, > > + securityfs_create_file(name, mode, parent, (u8 *)(uintptr_t)key, > > &tomoyo_operations); > > (1) Does clang warn if "(void *)key" is used instead of "(u8 *)(uintptr_t)key" ? Yes, both clang and gcc warn when you cast between a pointer and an integer of a different size. > (2) tomoyo_open() has > > const int key = ((u8 *) file_inode(file)->i_private) - ((u8 *) NULL); > > which decodes the "u8 key" passed to tomoyo_create_entry(). For symmetry, > I'd like to remove NULL from tomoyo_open() as well. Does clang warn if > > const int key = (u8) (file_inode(file)->i_private); > > is used? Yes, same thing, but const int key = (uintptr_t)file_inode(file)->i_private; works without warnings and seems clearer. Arnd
OK. I will apply the following patch to my tree. Thank you. From: Arnd Bergmann <arnd@arndb.de> Subject: [PATCH] tomoyo: fix clang pointer arithmetic warning clang warns about additions on NULL pointers being undefined in C: security/tomoyo/securityfs_if.c:226:59: warning: arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension [-Wnull-pointer-arithmetic] securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, Change the code to instead use a cast through uintptr_t to avoid the warning. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- security/tomoyo/securityfs_if.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/security/tomoyo/securityfs_if.c b/security/tomoyo/securityfs_if.c index 546281c5b233..065f4941c4d8 100644 --- a/security/tomoyo/securityfs_if.c +++ b/security/tomoyo/securityfs_if.c @@ -131,8 +131,8 @@ static const struct file_operations tomoyo_self_operations = { */ static int tomoyo_open(struct inode *inode, struct file *file) { - const int key = ((u8 *) file_inode(file)->i_private) - - ((u8 *) NULL); + const u8 key = (uintptr_t) file_inode(file)->i_private; + return tomoyo_open_control(key, file); } @@ -223,7 +223,7 @@ static const struct file_operations tomoyo_operations = { static void __init tomoyo_create_entry(const char *name, const umode_t mode, struct dentry *parent, const u8 key) { - securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, + securityfs_create_file(name, mode, parent, (void *) (uintptr_t) key, &tomoyo_operations); }
On Mon, Oct 26, 2020 at 2:52 PM Arnd Bergmann <arnd@kernel.org> wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > clang warns about additions on NULL pointers being undefined in C: > > security/tomoyo/securityfs_if.c:226:59: warning: arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension [-Wnull-pointer-arithmetic] > securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, > > Change the code to instead use a cast through uintptr_t to avoid > the warning. > > Fixes: 9590837b89aa ("Common functions for TOMOYO Linux.") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > security/tomoyo/securityfs_if.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/tomoyo/securityfs_if.c b/security/tomoyo/securityfs_if.c > index 546281c5b233..0a5f00073ef1 100644 > --- a/security/tomoyo/securityfs_if.c > +++ b/security/tomoyo/securityfs_if.c > @@ -223,7 +223,7 @@ static const struct file_operations tomoyo_operations = { > static void __init tomoyo_create_entry(const char *name, const umode_t mode, > struct dentry *parent, const u8 key) > { > - securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, > + securityfs_create_file(name, mode, parent, (u8 *)(uintptr_t)key, > &tomoyo_operations); > } > > -- > 2.27.0 >
diff --git a/security/tomoyo/securityfs_if.c b/security/tomoyo/securityfs_if.c index 546281c5b233..0a5f00073ef1 100644 --- a/security/tomoyo/securityfs_if.c +++ b/security/tomoyo/securityfs_if.c @@ -223,7 +223,7 @@ static const struct file_operations tomoyo_operations = { static void __init tomoyo_create_entry(const char *name, const umode_t mode, struct dentry *parent, const u8 key) { - securityfs_create_file(name, mode, parent, ((u8 *) NULL) + key, + securityfs_create_file(name, mode, parent, (u8 *)(uintptr_t)key, &tomoyo_operations); }