DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations
@ 2020-10-16  5:11 kiransuren, kirank.suren
  2020-10-16  5:16 ` Randy Dunlap
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: kiransuren, kirank.suren @ 2020-10-16  5:11 UTC (permalink / raw)
  To: gregkh; +Cc: devel, kiransuren, linux-kernel

From: kiransuren <kirank.suren@gmail.com>

Fixed a coding style issue.

Signed-off-by: Kiran Surendran <kirank.suren@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 10b4be1f3e78..f2b67c4fe8d2 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -376,7 +376,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 file_operations vmfile_fops;
 	struct ashmem_area *asma = file->private_data;
 	int ret = 0;
 
-- 
2.17.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations
  2020-10-16  5:11 [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations kiransuren, kirank.suren
@ 2020-10-16  5:16 ` Randy Dunlap
  2020-10-16  5:17 ` Greg KH
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2020-10-16  5:16 UTC (permalink / raw)
  To: kiransuren, kirank.suren, gregkh; +Cc: devel, linux-kernel

On 10/15/20 10:11 PM, kiransuren@osuosl.org wrote:
> From: kiransuren <kirank.suren@gmail.com>
> 
> Fixed a coding style issue.
> 
> Signed-off-by: Kiran Surendran <kirank.suren@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 10b4be1f3e78..f2b67c4fe8d2 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -376,7 +376,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 file_operations vmfile_fops;
>  	struct ashmem_area *asma = file->private_data;
>  	int ret = 0;
>  
> 

Did you build/compile this change?  My brain syntax checker doesn't like it.

and what coding style issue?

-- 
~Randy

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations
  2020-10-16  5:11 [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations kiransuren, kirank.suren
  2020-10-16  5:16 ` Randy Dunlap
@ 2020-10-16  5:17 ` Greg KH
       [not found] ` <CAMHzSAHVtv+BuYgCAdbB26J3EHTdEyH3ZZUfP-g3zftppzJOkw@mail.gmail.com>
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-10-16  5:17 UTC (permalink / raw)
  To: kiransuren, kirank.suren; +Cc: devel, linux-kernel

On Fri, Oct 16, 2020 at 01:11:11AM -0400, kiransuren@osuosl.org wrote:
> From: kiransuren <kirank.suren@gmail.com>

This does not match your signed-off-by line :(

> 
> Fixed a coding style issue.

What issue?  Always be specific.

> 
> Signed-off-by: Kiran Surendran <kirank.suren@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 10b4be1f3e78..f2b67c4fe8d2 100644
> --- a/drivers/staging/android/ashmem.c
> +++ b/drivers/staging/android/ashmem.c
> @@ -376,7 +376,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 file_operations vmfile_fops;

Why did you not test-build your patch before sending it out?

Don't you think that is a big rude?

Please be more careful.

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations
       [not found] ` <CAMHzSAHVtv+BuYgCAdbB26J3EHTdEyH3ZZUfP-g3zftppzJOkw@mail.gmail.com>
@ 2020-10-16  6:18   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-10-16  6:18 UTC (permalink / raw)
  To: Kiran Suren; +Cc: devel, linux-kernel

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Fri, Oct 16, 2020 at 01:38:47AM -0400, Kiran Suren wrote:
> Hi,
> 
> My apologies about the last couple emails, I test built completely the
> wrong file. This is my first attempt at doing patch so I’m still trying to
> get a hang of how all this works. Also, I’m not sure what you mean by the
> signed off line? I believe I did put my actual full name? Again, apologies
> for any inconveniences just trying to get a hang of how the entire process
> works.

For the signed-off-by line, look at your email below:

> 
> On Fri, Oct 16, 2020 at 1:12 AM kiransuren <kirank.suren@gmail.com> wrote:
> 
> > From: kiransuren <kirank.suren@gmail.com>

That has one name and then:

> >
> > Fixed a coding style issue.
> >
> > Signed-off-by: Kiran Surendran <kirank.suren@gmail.com>

This has another.

They have to match, otherwise it doesn't make any sense, right?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations
  2020-10-16  5:11 [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations kiransuren, kirank.suren
                   ` (2 preceding siblings ...)
       [not found] ` <CAMHzSAHVtv+BuYgCAdbB26J3EHTdEyH3ZZUfP-g3zftppzJOkw@mail.gmail.com>
@ 2020-10-16  6:20 ` kernel test robot
  2020-10-16  7:34 ` kernel test robot
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-10-16  6:20 UTC (permalink / raw)
  To: kiransuren, kirank.suren, gregkh
  Cc: devel, kiransuren, kbuild-all, linux-kernel


[-- Attachment #1: Type: text/plain, Size: 5507 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/kiransuren-osuosl-org/Staging-android-ashmem-changed-struct-file_operations-to-const-file_operations/20201016-131238
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 726eb70e0d34dc4bc4dada71f52bba8ed638431e
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 9.3.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/1496e5f2103cc6f96af90aaf323cf92f018dcf41
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review kiransuren-osuosl-org/Staging-android-ashmem-changed-struct-file_operations-to-const-file_operations/20201016-131238
        git checkout 1496e5f2103cc6f96af90aaf323cf92f018dcf41
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa 

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:379:15: error: unknown type name 'file_operations'
     379 |  static const file_operations vmfile_fops;
         |               ^~~~~~~~~~~~~~~
>> drivers/staging/android/ashmem.c:429:19: error: request for member 'mmap' in something not a structure or union
     429 |   if (!vmfile_fops.mmap) {
         |                   ^
>> drivers/staging/android/ashmem.c:430:16: error: assignment of read-only variable 'vmfile_fops'
     430 |    vmfile_fops = *vmfile->f_op;
         |                ^
   drivers/staging/android/ashmem.c:431:15: error: request for member 'mmap' in something not a structure or union
     431 |    vmfile_fops.mmap = ashmem_vmfile_mmap;
         |               ^
>> drivers/staging/android/ashmem.c:432:15: error: request for member 'get_unmapped_area' in something not a structure or union
     432 |    vmfile_fops.get_unmapped_area =
         |               ^
>> drivers/staging/android/ashmem.c:435:16: error: assignment to 'const struct file_operations *' from incompatible pointer type 'const int *' [-Werror=incompatible-pointer-types]
     435 |   vmfile->f_op = &vmfile_fops;
         |                ^
   cc1: some warnings being treated as errors

vim +/file_operations +379 drivers/staging/android/ashmem.c

   376	
   377	static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
   378	{
 > 379		static const file_operations vmfile_fops;
   380		struct ashmem_area *asma = file->private_data;
   381		int ret = 0;
   382	
   383		mutex_lock(&ashmem_mutex);
   384	
   385		/* user needs to SET_SIZE before mapping */
   386		if (!asma->size) {
   387			ret = -EINVAL;
   388			goto out;
   389		}
   390	
   391		/* requested mapping size larger than object size */
   392		if (vma->vm_end - vma->vm_start > PAGE_ALIGN(asma->size)) {
   393			ret = -EINVAL;
   394			goto out;
   395		}
   396	
   397		/* requested protection bits must match our allowed protection mask */
   398		if ((vma->vm_flags & ~calc_vm_prot_bits(asma->prot_mask, 0)) &
   399		    calc_vm_prot_bits(PROT_MASK, 0)) {
   400			ret = -EPERM;
   401			goto out;
   402		}
   403		vma->vm_flags &= ~calc_vm_may_flags(~asma->prot_mask);
   404	
   405		if (!asma->file) {
   406			char *name = ASHMEM_NAME_DEF;
   407			struct file *vmfile;
   408			struct inode *inode;
   409	
   410			if (asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0')
   411				name = asma->name;
   412	
   413			/* ... and allocate the backing shmem file */
   414			vmfile = shmem_file_setup(name, asma->size, vma->vm_flags);
   415			if (IS_ERR(vmfile)) {
   416				ret = PTR_ERR(vmfile);
   417				goto out;
   418			}
   419			vmfile->f_mode |= FMODE_LSEEK;
   420			inode = file_inode(vmfile);
   421			lockdep_set_class(&inode->i_rwsem, &backing_shmem_inode_class);
   422			asma->file = vmfile;
   423			/*
   424			 * override mmap operation of the vmfile so that it can't be
   425			 * remapped which would lead to creation of a new vma with no
   426			 * asma permission checks. Have to override get_unmapped_area
   427			 * as well to prevent VM_BUG_ON check for f_ops modification.
   428			 */
 > 429			if (!vmfile_fops.mmap) {
 > 430				vmfile_fops = *vmfile->f_op;
   431				vmfile_fops.mmap = ashmem_vmfile_mmap;
 > 432				vmfile_fops.get_unmapped_area =
   433						ashmem_vmfile_get_unmapped_area;
   434			}
 > 435			vmfile->f_op = &vmfile_fops;
   436		}
   437		get_file(asma->file);
   438	
   439		/*
   440		 * XXX - Reworked to use shmem_zero_setup() instead of
   441		 * shmem_set_file while we're in staging. -jstultz
   442		 */
   443		if (vma->vm_flags & VM_SHARED) {
   444			ret = shmem_zero_setup(vma);
   445			if (ret) {
   446				fput(asma->file);
   447				goto out;
   448			}
   449		} else {
   450			vma_set_anonymous(vma);
   451		}
   452	
   453		if (vma->vm_file)
   454			fput(vma->vm_file);
   455		vma->vm_file = asma->file;
   456	
   457	out:
   458		mutex_unlock(&ashmem_mutex);
   459		return ret;
   460	}
   461	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65636 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations
  2020-10-16  5:11 [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations kiransuren, kirank.suren
                   ` (3 preceding siblings ...)
  2020-10-16  6:20 ` kernel test robot
@ 2020-10-16  7:34 ` kernel test robot
  4 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2020-10-16  7:34 UTC (permalink / raw)
  To: kiransuren, kirank.suren, gregkh
  Cc: devel, kiransuren, kbuild-all, linux-kernel


[-- Attachment #1: Type: text/plain, Size: 5999 bytes --]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/kiransuren-osuosl-org/Staging-android-ashmem-changed-struct-file_operations-to-const-file_operations/20201016-131238
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 726eb70e0d34dc4bc4dada71f52bba8ed638431e
config: x86_64-randconfig-a006-20201016 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5fbab4025eb57b12f2842ab188ff07a110708e1d)
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
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/1496e5f2103cc6f96af90aaf323cf92f018dcf41
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review kiransuren-osuosl-org/Staging-android-ashmem-changed-struct-file_operations-to-const-file_operations/20201016-131238
        git checkout 1496e5f2103cc6f96af90aaf323cf92f018dcf41
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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:379:15: error: must use 'struct' tag to refer to type 'file_operations'
           static const file_operations vmfile_fops;
                        ^
                        struct 
>> drivers/staging/android/ashmem.c:430:16: error: cannot assign to variable 'vmfile_fops' with const-qualified type 'const struct file_operations'
                           vmfile_fops = *vmfile->f_op;
                           ~~~~~~~~~~~ ^
   drivers/staging/android/ashmem.c:379:31: note: variable 'vmfile_fops' declared const here
           static const file_operations vmfile_fops;
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   drivers/staging/android/ashmem.c:431:21: error: cannot assign to variable 'vmfile_fops' with const-qualified type 'const struct file_operations'
                           vmfile_fops.mmap = ashmem_vmfile_mmap;
                           ~~~~~~~~~~~~~~~~ ^
   drivers/staging/android/ashmem.c:379:31: note: variable 'vmfile_fops' declared const here
           static const file_operations vmfile_fops;
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   drivers/staging/android/ashmem.c:432:34: error: cannot assign to variable 'vmfile_fops' with const-qualified type 'const struct file_operations'
                           vmfile_fops.get_unmapped_area =
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
   drivers/staging/android/ashmem.c:379:31: note: variable 'vmfile_fops' declared const here
           static const file_operations vmfile_fops;
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   4 errors generated.

vim +379 drivers/staging/android/ashmem.c

   376	
   377	static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
   378	{
 > 379		static const file_operations vmfile_fops;
   380		struct ashmem_area *asma = file->private_data;
   381		int ret = 0;
   382	
   383		mutex_lock(&ashmem_mutex);
   384	
   385		/* user needs to SET_SIZE before mapping */
   386		if (!asma->size) {
   387			ret = -EINVAL;
   388			goto out;
   389		}
   390	
   391		/* requested mapping size larger than object size */
   392		if (vma->vm_end - vma->vm_start > PAGE_ALIGN(asma->size)) {
   393			ret = -EINVAL;
   394			goto out;
   395		}
   396	
   397		/* requested protection bits must match our allowed protection mask */
   398		if ((vma->vm_flags & ~calc_vm_prot_bits(asma->prot_mask, 0)) &
   399		    calc_vm_prot_bits(PROT_MASK, 0)) {
   400			ret = -EPERM;
   401			goto out;
   402		}
   403		vma->vm_flags &= ~calc_vm_may_flags(~asma->prot_mask);
   404	
   405		if (!asma->file) {
   406			char *name = ASHMEM_NAME_DEF;
   407			struct file *vmfile;
   408			struct inode *inode;
   409	
   410			if (asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0')
   411				name = asma->name;
   412	
   413			/* ... and allocate the backing shmem file */
   414			vmfile = shmem_file_setup(name, asma->size, vma->vm_flags);
   415			if (IS_ERR(vmfile)) {
   416				ret = PTR_ERR(vmfile);
   417				goto out;
   418			}
   419			vmfile->f_mode |= FMODE_LSEEK;
   420			inode = file_inode(vmfile);
   421			lockdep_set_class(&inode->i_rwsem, &backing_shmem_inode_class);
   422			asma->file = vmfile;
   423			/*
   424			 * override mmap operation of the vmfile so that it can't be
   425			 * remapped which would lead to creation of a new vma with no
   426			 * asma permission checks. Have to override get_unmapped_area
   427			 * as well to prevent VM_BUG_ON check for f_ops modification.
   428			 */
   429			if (!vmfile_fops.mmap) {
 > 430				vmfile_fops = *vmfile->f_op;
   431				vmfile_fops.mmap = ashmem_vmfile_mmap;
   432				vmfile_fops.get_unmapped_area =
   433						ashmem_vmfile_get_unmapped_area;
   434			}
   435			vmfile->f_op = &vmfile_fops;
   436		}
   437		get_file(asma->file);
   438	
   439		/*
   440		 * XXX - Reworked to use shmem_zero_setup() instead of
   441		 * shmem_set_file while we're in staging. -jstultz
   442		 */
   443		if (vma->vm_flags & VM_SHARED) {
   444			ret = shmem_zero_setup(vma);
   445			if (ret) {
   446				fput(asma->file);
   447				goto out;
   448			}
   449		} else {
   450			vma_set_anonymous(vma);
   451		}
   452	
   453		if (vma->vm_file)
   454			fput(vma->vm_file);
   455		vma->vm_file = asma->file;
   456	
   457	out:
   458		mutex_unlock(&ashmem_mutex);
   459		return ret;
   460	}
   461	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33882 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16  5:11 [PATCH] Staging: android: ashmem: changed struct file_operations to const file_operations kiransuren, kirank.suren
2020-10-16  5:16 ` Randy Dunlap
2020-10-16  5:17 ` Greg KH
     [not found] ` <CAMHzSAHVtv+BuYgCAdbB26J3EHTdEyH3ZZUfP-g3zftppzJOkw@mail.gmail.com>
2020-10-16  6:18   ` Greg KH
2020-10-16  6:20 ` kernel test robot
2020-10-16  7:34 ` kernel test robot

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org
	public-inbox-index driverdev-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git