* [PATCH] Staging : android: Struct file_operations should be const
@ 2022-02-07 3:17 Leonardo Araujo
2022-02-07 6:32 ` Joe Perches
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Leonardo Araujo @ 2022-02-07 3:17 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, linux-staging, Leonardo Araujo
From: "Leonardo Araujo" <leonardo.aa88@gmail.com>
WARNING: struct file_operations should normally be const
Signed-off-by: Leonardo Araujo <leonardo.aa88@gmail.com>
---
drivers/staging/android/ashmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index ddbde3f8430e..4c6b420fbf4d 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -377,7 +377,7 @@ ashmem_vmfile_get_unmapped_area(struct file *file, unsigned long addr,
static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
{
- static struct file_operations vmfile_fops;
+ static const struct file_operations vmfile_fops;
struct ashmem_area *asma = file->private_data;
int ret = 0;
--
2.29.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] Staging : android: Struct file_operations should be const 2022-02-07 3:17 [PATCH] Staging : android: Struct file_operations should be const Leonardo Araujo @ 2022-02-07 6:32 ` Joe Perches 2022-02-07 8:19 ` kernel test robot 2022-02-07 16:01 ` Al Viro 2 siblings, 0 replies; 6+ messages in thread From: Joe Perches @ 2022-02-07 6:32 UTC (permalink / raw) To: Leonardo Araujo, gregkh; +Cc: linux-kernel, linux-staging On Mon, 2022-02-07 at 00:17 -0300, Leonardo Araujo wrote: > From: "Leonardo Araujo" <leonardo.aa88@gmail.com> > > WARNING: struct file_operations should normally be const Always compile the files modified by a patch before you send it out. [] > diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c [] > @@ -377,7 +377,7 @@ ashmem_vmfile_get_unmapped_area(struct file *file, unsigned long addr, > > static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) > { > - static struct file_operations vmfile_fops; > + static const struct file_operations vmfile_fops; > struct ashmem_area *asma = file->private_data; > int ret = 0; > <apply patch> $ make allyesconfig $ make drivers/staging/android/ashmem.o [...] CC drivers/staging/android/ashmem.o drivers/staging/android/ashmem.c: In function ‘ashmem_mmap’: drivers/staging/android/ashmem.c:431:37: error: assignment of read-only variable ‘vmfile_fops’ 431 | vmfile_fops = *vmfile->f_op; | ^ drivers/staging/android/ashmem.c:432:42: error: assignment of member ‘mmap’ in read-only object 432 | vmfile_fops.mmap = ashmem_vmfile_mmap; | ^ drivers/staging/android/ashmem.c:433:55: error: assignment of member ‘get_unmapped_area’ in read-only object 433 | vmfile_fops.get_unmapped_area = | ^ make[3]: *** [scripts/Makefile.build:289: drivers/staging/android/ashmem.o] Error 1 make[2]: *** [scripts/Makefile.build:572: drivers/staging/android] Error 2 make[1]: *** [scripts/Makefile.build:572: drivers/staging] Error 2 make: *** [Makefile:1965: drivers] Error 2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging : android: Struct file_operations should be const 2022-02-07 3:17 [PATCH] Staging : android: Struct file_operations should be const Leonardo Araujo @ 2022-02-07 8:19 ` kernel test robot 2022-02-07 8:19 ` kernel test robot 2022-02-07 16:01 ` Al Viro 2 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2022-02-07 8:19 UTC (permalink / raw) To: Leonardo Araujo, gregkh Cc: kbuild-all, linux-kernel, linux-staging, Leonardo Araujo Hi Leonardo, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on linux/master linus/master v5.17-rc3 next-20220204] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Leonardo-Araujo/Staging-android-Struct-file_operations-should-be-const/20220207-140301 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 74e69e95f13f91c57c9322c7754a85a0e13e9b90 config: s390-randconfig-r044-20220207 (https://download.01.org/0day-ci/archive/20220207/202202071653.db3wZIpH-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/64cb13193a6a834c5cc3118068cf0ea5670bae8d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Leonardo-Araujo/Staging-android-Struct-file_operations-should-be-const/20220207-140301 git checkout 64cb13193a6a834c5cc3118068cf0ea5670bae8d # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash drivers/staging/android/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/staging/android/ashmem.c: In function 'ashmem_mmap': >> drivers/staging/android/ashmem.c:431:37: error: assignment of read-only variable 'vmfile_fops' 431 | vmfile_fops = *vmfile->f_op; | ^ >> drivers/staging/android/ashmem.c:432:42: error: assignment of member 'mmap' in read-only object 432 | vmfile_fops.mmap = ashmem_vmfile_mmap; | ^ >> drivers/staging/android/ashmem.c:433:55: error: assignment of member 'get_unmapped_area' in read-only object 433 | vmfile_fops.get_unmapped_area = | ^ vim +/vmfile_fops +431 drivers/staging/android/ashmem.c 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 377 11980c2ac4ccfa Robert Love 2011-12-20 378 static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) 11980c2ac4ccfa Robert Love 2011-12-20 379 { 64cb13193a6a83 Leonardo Araujo 2022-02-07 380 static const struct file_operations vmfile_fops; 11980c2ac4ccfa Robert Love 2011-12-20 381 struct ashmem_area *asma = file->private_data; 11980c2ac4ccfa Robert Love 2011-12-20 382 int ret = 0; 11980c2ac4ccfa Robert Love 2011-12-20 383 11980c2ac4ccfa Robert Love 2011-12-20 384 mutex_lock(&ashmem_mutex); 11980c2ac4ccfa Robert Love 2011-12-20 385 11980c2ac4ccfa Robert Love 2011-12-20 386 /* user needs to SET_SIZE before mapping */ 59848d6aded59a Alistair Strachan 2018-06-19 387 if (!asma->size) { 11980c2ac4ccfa Robert Love 2011-12-20 388 ret = -EINVAL; 11980c2ac4ccfa Robert Love 2011-12-20 389 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 390 } 11980c2ac4ccfa Robert Love 2011-12-20 391 8632c614565d0c Alistair Strachan 2018-06-19 392 /* requested mapping size larger than object size */ 8632c614565d0c Alistair Strachan 2018-06-19 393 if (vma->vm_end - vma->vm_start > PAGE_ALIGN(asma->size)) { 11980c2ac4ccfa Robert Love 2011-12-20 394 ret = -EINVAL; 11980c2ac4ccfa Robert Love 2011-12-20 395 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 396 } 11980c2ac4ccfa Robert Love 2011-12-20 397 11980c2ac4ccfa Robert Love 2011-12-20 398 /* requested protection bits must match our allowed protection mask */ 59848d6aded59a Alistair Strachan 2018-06-19 399 if ((vma->vm_flags & ~calc_vm_prot_bits(asma->prot_mask, 0)) & 59848d6aded59a Alistair Strachan 2018-06-19 400 calc_vm_prot_bits(PROT_MASK, 0)) { 11980c2ac4ccfa Robert Love 2011-12-20 401 ret = -EPERM; 11980c2ac4ccfa Robert Love 2011-12-20 402 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 403 } 56f76fc68492af Arve Hjønnevåg 2011-12-20 404 vma->vm_flags &= ~calc_vm_may_flags(~asma->prot_mask); 11980c2ac4ccfa Robert Love 2011-12-20 405 11980c2ac4ccfa Robert Love 2011-12-20 406 if (!asma->file) { 11980c2ac4ccfa Robert Love 2011-12-20 407 char *name = ASHMEM_NAME_DEF; 11980c2ac4ccfa Robert Love 2011-12-20 408 struct file *vmfile; 3e338d3c95c735 Suren Baghdasaryan 2020-07-30 409 struct inode *inode; 11980c2ac4ccfa Robert Love 2011-12-20 410 11980c2ac4ccfa Robert Love 2011-12-20 411 if (asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0') 11980c2ac4ccfa Robert Love 2011-12-20 412 name = asma->name; 11980c2ac4ccfa Robert Love 2011-12-20 413 11980c2ac4ccfa Robert Love 2011-12-20 414 /* ... and allocate the backing shmem file */ 11980c2ac4ccfa Robert Love 2011-12-20 415 vmfile = shmem_file_setup(name, asma->size, vma->vm_flags); 7f44cb0ba88b40 Viresh Kumar 2015-07-31 416 if (IS_ERR(vmfile)) { 11980c2ac4ccfa Robert Love 2011-12-20 417 ret = PTR_ERR(vmfile); 11980c2ac4ccfa Robert Love 2011-12-20 418 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 419 } 97fbfef6bd5978 Shuxiao Zhang 2017-04-06 420 vmfile->f_mode |= FMODE_LSEEK; 3e338d3c95c735 Suren Baghdasaryan 2020-07-30 421 inode = file_inode(vmfile); 3e338d3c95c735 Suren Baghdasaryan 2020-07-30 422 lockdep_set_class(&inode->i_rwsem, &backing_shmem_inode_class); 11980c2ac4ccfa Robert Love 2011-12-20 423 asma->file = vmfile; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 424 /* 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 425 * override mmap operation of the vmfile so that it can't be 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 426 * remapped which would lead to creation of a new vma with no 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 427 * asma permission checks. Have to override get_unmapped_area 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 428 * as well to prevent VM_BUG_ON check for f_ops modification. 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 429 */ 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 430 if (!vmfile_fops.mmap) { 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 @431 vmfile_fops = *vmfile->f_op; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 @432 vmfile_fops.mmap = ashmem_vmfile_mmap; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 @433 vmfile_fops.get_unmapped_area = 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 434 ashmem_vmfile_get_unmapped_area; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 435 } 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 436 vmfile->f_op = &vmfile_fops; 11980c2ac4ccfa Robert Love 2011-12-20 437 } 11980c2ac4ccfa Robert Love 2011-12-20 438 get_file(asma->file); 11980c2ac4ccfa Robert Love 2011-12-20 439 11980c2ac4ccfa Robert Love 2011-12-20 440 /* 11980c2ac4ccfa Robert Love 2011-12-20 441 * XXX - Reworked to use shmem_zero_setup() instead of 11980c2ac4ccfa Robert Love 2011-12-20 442 * shmem_set_file while we're in staging. -jstultz 11980c2ac4ccfa Robert Love 2011-12-20 443 */ 11980c2ac4ccfa Robert Love 2011-12-20 444 if (vma->vm_flags & VM_SHARED) { 11980c2ac4ccfa Robert Love 2011-12-20 445 ret = shmem_zero_setup(vma); 11980c2ac4ccfa Robert Love 2011-12-20 446 if (ret) { 11980c2ac4ccfa Robert Love 2011-12-20 447 fput(asma->file); 11980c2ac4ccfa Robert Love 2011-12-20 448 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 449 } 44960f2a7b63e2 John Stultz 2018-07-31 450 } else { 44960f2a7b63e2 John Stultz 2018-07-31 451 vma_set_anonymous(vma); 11980c2ac4ccfa Robert Love 2011-12-20 452 } 11980c2ac4ccfa Robert Love 2011-12-20 453 295992fb815e79 Christian König 2020-09-14 454 vma_set_file(vma, asma->file); 295992fb815e79 Christian König 2020-09-14 455 /* XXX: merge this with the get_file() above if possible */ 295992fb815e79 Christian König 2020-09-14 456 fput(asma->file); 11980c2ac4ccfa Robert Love 2011-12-20 457 11980c2ac4ccfa Robert Love 2011-12-20 458 out: 11980c2ac4ccfa Robert Love 2011-12-20 459 mutex_unlock(&ashmem_mutex); 11980c2ac4ccfa Robert Love 2011-12-20 460 return ret; 11980c2ac4ccfa Robert Love 2011-12-20 461 } 11980c2ac4ccfa Robert Love 2011-12-20 462 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging : android: Struct file_operations should be const @ 2022-02-07 8:19 ` kernel test robot 0 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2022-02-07 8:19 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 9299 bytes --] Hi Leonardo, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on linux/master linus/master v5.17-rc3 next-20220204] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Leonardo-Araujo/Staging-android-Struct-file_operations-should-be-const/20220207-140301 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 74e69e95f13f91c57c9322c7754a85a0e13e9b90 config: s390-randconfig-r044-20220207 (https://download.01.org/0day-ci/archive/20220207/202202071653.db3wZIpH-lkp(a)intel.com/config) compiler: s390-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/64cb13193a6a834c5cc3118068cf0ea5670bae8d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Leonardo-Araujo/Staging-android-Struct-file_operations-should-be-const/20220207-140301 git checkout 64cb13193a6a834c5cc3118068cf0ea5670bae8d # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=s390 SHELL=/bin/bash drivers/staging/android/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/staging/android/ashmem.c: In function 'ashmem_mmap': >> drivers/staging/android/ashmem.c:431:37: error: assignment of read-only variable 'vmfile_fops' 431 | vmfile_fops = *vmfile->f_op; | ^ >> drivers/staging/android/ashmem.c:432:42: error: assignment of member 'mmap' in read-only object 432 | vmfile_fops.mmap = ashmem_vmfile_mmap; | ^ >> drivers/staging/android/ashmem.c:433:55: error: assignment of member 'get_unmapped_area' in read-only object 433 | vmfile_fops.get_unmapped_area = | ^ vim +/vmfile_fops +431 drivers/staging/android/ashmem.c 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 377 11980c2ac4ccfa Robert Love 2011-12-20 378 static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) 11980c2ac4ccfa Robert Love 2011-12-20 379 { 64cb13193a6a83 Leonardo Araujo 2022-02-07 380 static const struct file_operations vmfile_fops; 11980c2ac4ccfa Robert Love 2011-12-20 381 struct ashmem_area *asma = file->private_data; 11980c2ac4ccfa Robert Love 2011-12-20 382 int ret = 0; 11980c2ac4ccfa Robert Love 2011-12-20 383 11980c2ac4ccfa Robert Love 2011-12-20 384 mutex_lock(&ashmem_mutex); 11980c2ac4ccfa Robert Love 2011-12-20 385 11980c2ac4ccfa Robert Love 2011-12-20 386 /* user needs to SET_SIZE before mapping */ 59848d6aded59a Alistair Strachan 2018-06-19 387 if (!asma->size) { 11980c2ac4ccfa Robert Love 2011-12-20 388 ret = -EINVAL; 11980c2ac4ccfa Robert Love 2011-12-20 389 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 390 } 11980c2ac4ccfa Robert Love 2011-12-20 391 8632c614565d0c Alistair Strachan 2018-06-19 392 /* requested mapping size larger than object size */ 8632c614565d0c Alistair Strachan 2018-06-19 393 if (vma->vm_end - vma->vm_start > PAGE_ALIGN(asma->size)) { 11980c2ac4ccfa Robert Love 2011-12-20 394 ret = -EINVAL; 11980c2ac4ccfa Robert Love 2011-12-20 395 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 396 } 11980c2ac4ccfa Robert Love 2011-12-20 397 11980c2ac4ccfa Robert Love 2011-12-20 398 /* requested protection bits must match our allowed protection mask */ 59848d6aded59a Alistair Strachan 2018-06-19 399 if ((vma->vm_flags & ~calc_vm_prot_bits(asma->prot_mask, 0)) & 59848d6aded59a Alistair Strachan 2018-06-19 400 calc_vm_prot_bits(PROT_MASK, 0)) { 11980c2ac4ccfa Robert Love 2011-12-20 401 ret = -EPERM; 11980c2ac4ccfa Robert Love 2011-12-20 402 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 403 } 56f76fc68492af Arve Hjønnevåg 2011-12-20 404 vma->vm_flags &= ~calc_vm_may_flags(~asma->prot_mask); 11980c2ac4ccfa Robert Love 2011-12-20 405 11980c2ac4ccfa Robert Love 2011-12-20 406 if (!asma->file) { 11980c2ac4ccfa Robert Love 2011-12-20 407 char *name = ASHMEM_NAME_DEF; 11980c2ac4ccfa Robert Love 2011-12-20 408 struct file *vmfile; 3e338d3c95c735 Suren Baghdasaryan 2020-07-30 409 struct inode *inode; 11980c2ac4ccfa Robert Love 2011-12-20 410 11980c2ac4ccfa Robert Love 2011-12-20 411 if (asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0') 11980c2ac4ccfa Robert Love 2011-12-20 412 name = asma->name; 11980c2ac4ccfa Robert Love 2011-12-20 413 11980c2ac4ccfa Robert Love 2011-12-20 414 /* ... and allocate the backing shmem file */ 11980c2ac4ccfa Robert Love 2011-12-20 415 vmfile = shmem_file_setup(name, asma->size, vma->vm_flags); 7f44cb0ba88b40 Viresh Kumar 2015-07-31 416 if (IS_ERR(vmfile)) { 11980c2ac4ccfa Robert Love 2011-12-20 417 ret = PTR_ERR(vmfile); 11980c2ac4ccfa Robert Love 2011-12-20 418 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 419 } 97fbfef6bd5978 Shuxiao Zhang 2017-04-06 420 vmfile->f_mode |= FMODE_LSEEK; 3e338d3c95c735 Suren Baghdasaryan 2020-07-30 421 inode = file_inode(vmfile); 3e338d3c95c735 Suren Baghdasaryan 2020-07-30 422 lockdep_set_class(&inode->i_rwsem, &backing_shmem_inode_class); 11980c2ac4ccfa Robert Love 2011-12-20 423 asma->file = vmfile; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 424 /* 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 425 * override mmap operation of the vmfile so that it can't be 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 426 * remapped which would lead to creation of a new vma with no 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 427 * asma permission checks. Have to override get_unmapped_area 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 428 * as well to prevent VM_BUG_ON check for f_ops modification. 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 429 */ 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 430 if (!vmfile_fops.mmap) { 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 @431 vmfile_fops = *vmfile->f_op; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 @432 vmfile_fops.mmap = ashmem_vmfile_mmap; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 @433 vmfile_fops.get_unmapped_area = 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 434 ashmem_vmfile_get_unmapped_area; 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 435 } 6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 436 vmfile->f_op = &vmfile_fops; 11980c2ac4ccfa Robert Love 2011-12-20 437 } 11980c2ac4ccfa Robert Love 2011-12-20 438 get_file(asma->file); 11980c2ac4ccfa Robert Love 2011-12-20 439 11980c2ac4ccfa Robert Love 2011-12-20 440 /* 11980c2ac4ccfa Robert Love 2011-12-20 441 * XXX - Reworked to use shmem_zero_setup() instead of 11980c2ac4ccfa Robert Love 2011-12-20 442 * shmem_set_file while we're in staging. -jstultz 11980c2ac4ccfa Robert Love 2011-12-20 443 */ 11980c2ac4ccfa Robert Love 2011-12-20 444 if (vma->vm_flags & VM_SHARED) { 11980c2ac4ccfa Robert Love 2011-12-20 445 ret = shmem_zero_setup(vma); 11980c2ac4ccfa Robert Love 2011-12-20 446 if (ret) { 11980c2ac4ccfa Robert Love 2011-12-20 447 fput(asma->file); 11980c2ac4ccfa Robert Love 2011-12-20 448 goto out; 11980c2ac4ccfa Robert Love 2011-12-20 449 } 44960f2a7b63e2 John Stultz 2018-07-31 450 } else { 44960f2a7b63e2 John Stultz 2018-07-31 451 vma_set_anonymous(vma); 11980c2ac4ccfa Robert Love 2011-12-20 452 } 11980c2ac4ccfa Robert Love 2011-12-20 453 295992fb815e79 Christian König 2020-09-14 454 vma_set_file(vma, asma->file); 295992fb815e79 Christian König 2020-09-14 455 /* XXX: merge this with the get_file() above if possible */ 295992fb815e79 Christian König 2020-09-14 456 fput(asma->file); 11980c2ac4ccfa Robert Love 2011-12-20 457 11980c2ac4ccfa Robert Love 2011-12-20 458 out: 11980c2ac4ccfa Robert Love 2011-12-20 459 mutex_unlock(&ashmem_mutex); 11980c2ac4ccfa Robert Love 2011-12-20 460 return ret; 11980c2ac4ccfa Robert Love 2011-12-20 461 } 11980c2ac4ccfa Robert Love 2011-12-20 462 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging : android: Struct file_operations should be const 2022-02-07 3:17 [PATCH] Staging : android: Struct file_operations should be const Leonardo Araujo 2022-02-07 6:32 ` Joe Perches 2022-02-07 8:19 ` kernel test robot @ 2022-02-07 16:01 ` Al Viro 2022-02-08 9:48 ` Greg KH 2 siblings, 1 reply; 6+ messages in thread From: Al Viro @ 2022-02-07 16:01 UTC (permalink / raw) To: Leonardo Araujo; +Cc: gregkh, linux-kernel, linux-staging On Mon, Feb 07, 2022 at 12:17:11AM -0300, Leonardo Araujo wrote: > From: "Leonardo Araujo" <leonardo.aa88@gmail.com> > > WARNING: struct file_operations should normally be const > > Signed-off-by: Leonardo Araujo <leonardo.aa88@gmail.com> > --- > drivers/staging/android/ashmem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c > index ddbde3f8430e..4c6b420fbf4d 100644 > --- a/drivers/staging/android/ashmem.c > +++ b/drivers/staging/android/ashmem.c > @@ -377,7 +377,7 @@ ashmem_vmfile_get_unmapped_area(struct file *file, unsigned long addr, > > static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) > { > - static struct file_operations vmfile_fops; > + static const struct file_operations vmfile_fops; > struct ashmem_area *asma = file->private_data; > int ret = 0; Wait a minute. Why the hell would it possibly want a private instance of all-NULLs file_operations? Odd... <checks> if (!vmfile_fops.mmap) { vmfile_fops = *vmfile->f_op; vmfile_fops.mmap = ashmem_vmfile_mmap; vmfile_fops.get_unmapped_area = ashmem_vmfile_get_unmapped_area; } Er... So it *is* modified down the road. What, in your opinion, is signified by the const you are adding? Folks, could we please have the first "WARNING" in checkpatch.pl output replaced with "I'm a dumb script; this line looks like there might be something fishy in the area. Somebody smarter than me might want to take a look and figure out if there's something wrong going on there. From now on I'll mark all such places with 'WARNING' (with the summary of heuristics that pointed to them), to avoid repeating the above". Pretty please? This exact trap keeps snagging newbies - folks misinterpret "this place might be worth looking into" for "great (s)tool says: this is what's wrong there; must propitiate the great (s)tool!" In this case the damage is minimal - the resulting change would be instantly caught by compiler, so it's just a matter of mild embarrassment for poster. In other cases results had been nowhere near as mild. Incidentally, the place those heuristics had pointed too _DOES_ look fishy, indeed. What happens, AFAICS, is that the first time we hit that branch (asma->file being NULL) we stash a copy of whatever file_operations we get on file obtained by shmem_setup_file() (IOW, shmem_file_operations), with ->mmap and ->get_unmapped_area replaced with local functions. This is a bloody convoluted way to do things, not to mention being rather brittle... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Staging : android: Struct file_operations should be const 2022-02-07 16:01 ` Al Viro @ 2022-02-08 9:48 ` Greg KH 0 siblings, 0 replies; 6+ messages in thread From: Greg KH @ 2022-02-08 9:48 UTC (permalink / raw) To: Al Viro; +Cc: Leonardo Araujo, linux-kernel, linux-staging On Mon, Feb 07, 2022 at 04:01:01PM +0000, Al Viro wrote: > On Mon, Feb 07, 2022 at 12:17:11AM -0300, Leonardo Araujo wrote: > > From: "Leonardo Araujo" <leonardo.aa88@gmail.com> > > > > WARNING: struct file_operations should normally be const > > > > Signed-off-by: Leonardo Araujo <leonardo.aa88@gmail.com> > > --- > > drivers/staging/android/ashmem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c > > index ddbde3f8430e..4c6b420fbf4d 100644 > > --- a/drivers/staging/android/ashmem.c > > +++ b/drivers/staging/android/ashmem.c > > @@ -377,7 +377,7 @@ ashmem_vmfile_get_unmapped_area(struct file *file, unsigned long addr, > > > > static int ashmem_mmap(struct file *file, struct vm_area_struct *vma) > > { > > - static struct file_operations vmfile_fops; > > + static const struct file_operations vmfile_fops; > > struct ashmem_area *asma = file->private_data; > > int ret = 0; > > Wait a minute. Why the hell would it possibly want a private instance > of all-NULLs file_operations? Odd... > <checks> > if (!vmfile_fops.mmap) { > vmfile_fops = *vmfile->f_op; > vmfile_fops.mmap = ashmem_vmfile_mmap; > vmfile_fops.get_unmapped_area = > ashmem_vmfile_get_unmapped_area; > } > Er... So it *is* modified down the road. What, in your opinion, is signified > by the const you are adding? > > Folks, could we please have the first "WARNING" in checkpatch.pl output replaced > with > "I'm a dumb script; this line looks like there might be something fishy in the > area. Somebody smarter than me might want to take a look and figure out if > there's something wrong going on there. From now on I'll mark all such places > with 'WARNING' (with the summary of heuristics that pointed to them), to avoid > repeating the above". > > Pretty please? This exact trap keeps snagging newbies - folks misinterpret > "this place might be worth looking into" for "great (s)tool says: this is > what's wrong there; must propitiate the great (s)tool!" > > In this case the damage is minimal - the resulting change would be instantly > caught by compiler, so it's just a matter of mild embarrassment for poster. > In other cases results had been nowhere near as mild. It's a great "catch" for people who try to modify the kernel and then never actually test-build their changes. So for that reason alone I like it as it does reinforce the need for drive-by people to at least build the kernel tree. > Incidentally, the place those heuristics had pointed too _DOES_ look fishy, > indeed. What happens, AFAICS, is that the first time we hit that branch > (asma->file being NULL) we stash a copy of whatever file_operations we get > on file obtained by shmem_setup_file() (IOW, shmem_file_operations), > with ->mmap and ->get_unmapped_area replaced with local functions. > This is a bloody convoluted way to do things, not to mention being rather > brittle... > Ashmem is horrid and is not used by the Android project anymore, except by really old userspace programs. I think we could just delete it entirely now, I'll go ask the Android developers about it. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-08 9:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-07 3:17 [PATCH] Staging : android: Struct file_operations should be const Leonardo Araujo 2022-02-07 6:32 ` Joe Perches 2022-02-07 8:19 ` kernel test robot 2022-02-07 8:19 ` kernel test robot 2022-02-07 16:01 ` Al Viro 2022-02-08 9:48 ` Greg KH
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.