linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
@ 2022-06-17 23:04 kernel test robot
  2022-06-18  5:55 ` Charan Teja Kalla
  0 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2022-06-17 23:04 UTC (permalink / raw)
  To: Charan Teja Kalla
  Cc: kbuild-all, linux-kernel, Andrew Morton, Linux Memory Management List

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   4b35035bcf80ddb47c0112c4fbd84a63a2836a18
commit: 5bd009c7c9a9e888077c07535dc0c70aeab242c3 mm: madvise: return correct bytes advised with process_madvise
date:   3 months ago
compiler: mips-linux-gcc (GCC) 11.3.0
reproduce (cppcheck warning):
        # apt-get install cppcheck
        git checkout 5bd009c7c9a9e888077c07535dc0c70aeab242c3
        cppcheck --quiet --enable=style,performance,portability --template=gcc FILE

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


cppcheck warnings: (new ones prefixed by >>)
>> mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
    ret = (total_len - iov_iter_count(&iter)) ? : ret;
        ^

cppcheck possible warnings: (new ones prefixed by >>, may not real problems)

   mm/madvise.c:123:28: warning: Parameter 'anon_name' can be declared with const [constParameter]
        struct anon_vma_name *anon_name)
                              ^

vim +/ret +1438 mm/madvise.c

  1378	
  1379	SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec,
  1380			size_t, vlen, int, behavior, unsigned int, flags)
  1381	{
  1382		ssize_t ret;
  1383		struct iovec iovstack[UIO_FASTIOV], iovec;
  1384		struct iovec *iov = iovstack;
  1385		struct iov_iter iter;
  1386		struct task_struct *task;
  1387		struct mm_struct *mm;
  1388		size_t total_len;
  1389		unsigned int f_flags;
  1390	
  1391		if (flags != 0) {
  1392			ret = -EINVAL;
  1393			goto out;
  1394		}
  1395	
  1396		ret = import_iovec(READ, vec, vlen, ARRAY_SIZE(iovstack), &iov, &iter);
  1397		if (ret < 0)
  1398			goto out;
  1399	
  1400		task = pidfd_get_task(pidfd, &f_flags);
  1401		if (IS_ERR(task)) {
  1402			ret = PTR_ERR(task);
  1403			goto free_iov;
  1404		}
  1405	
  1406		if (!process_madvise_behavior_valid(behavior)) {
  1407			ret = -EINVAL;
  1408			goto release_task;
  1409		}
  1410	
  1411		/* Require PTRACE_MODE_READ to avoid leaking ASLR metadata. */
  1412		mm = mm_access(task, PTRACE_MODE_READ_FSCREDS);
  1413		if (IS_ERR_OR_NULL(mm)) {
  1414			ret = IS_ERR(mm) ? PTR_ERR(mm) : -ESRCH;
  1415			goto release_task;
  1416		}
  1417	
  1418		/*
  1419		 * Require CAP_SYS_NICE for influencing process performance. Note that
  1420		 * only non-destructive hints are currently supported.
  1421		 */
  1422		if (!capable(CAP_SYS_NICE)) {
  1423			ret = -EPERM;
  1424			goto release_mm;
  1425		}
  1426	
  1427		total_len = iov_iter_count(&iter);
  1428	
  1429		while (iov_iter_count(&iter)) {
  1430			iovec = iov_iter_iovec(&iter);
  1431			ret = do_madvise(mm, (unsigned long)iovec.iov_base,
  1432						iovec.iov_len, behavior);
  1433			if (ret < 0)
  1434				break;
  1435			iov_iter_advance(&iter, iovec.iov_len);
  1436		}
  1437	
> 1438		ret = (total_len - iov_iter_count(&iter)) ? : ret;

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


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

* Re: mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
  2022-06-17 23:04 mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment] kernel test robot
@ 2022-06-18  5:55 ` Charan Teja Kalla
  2022-06-20 16:29   ` Michal Hocko
  0 siblings, 1 reply; 6+ messages in thread
From: Charan Teja Kalla @ 2022-06-18  5:55 UTC (permalink / raw)
  To: kernel test robot, Michal Hocko, Minchan Kim
  Cc: kbuild-all, linux-kernel, Andrew Morton, Linux Memory Management List

Hello Andrew,

On 6/18/2022 4:34 AM, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   4b35035bcf80ddb47c0112c4fbd84a63a2836a18
> commit: 5bd009c7c9a9e888077c07535dc0c70aeab242c3 mm: madvise: return correct bytes advised with process_madvise
> date:   3 months ago
> compiler: mips-linux-gcc (GCC) 11.3.0
> reproduce (cppcheck warning):
>         # apt-get install cppcheck
>         git checkout 5bd009c7c9a9e888077c07535dc0c70aeab242c3
>         cppcheck --quiet --enable=style,performance,portability --template=gcc FILE
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> cppcheck warnings: (new ones prefixed by >>)
>>> mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
>     ret = (total_len - iov_iter_count(&iter)) ? : ret;

Other way to avoid this warning is by creating another local variable
that holds the total bytes processed. Having another local variable to
get rid off some compilation warning doesn't seem proper to me. So,
leaving this warning unless you ask me to fix this.

Thanks,
Charan


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

* Re: mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
  2022-06-18  5:55 ` Charan Teja Kalla
@ 2022-06-20 16:29   ` Michal Hocko
  2022-06-20 16:54     ` [kbuild-all] " Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2022-06-20 16:29 UTC (permalink / raw)
  To: Charan Teja Kalla
  Cc: kernel test robot, Minchan Kim, kbuild-all, linux-kernel,
	Andrew Morton, Linux Memory Management List

On Sat 18-06-22 11:25:43, Charan Teja Kalla wrote:
> Hello Andrew,
> 
> On 6/18/2022 4:34 AM, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head:   4b35035bcf80ddb47c0112c4fbd84a63a2836a18
> > commit: 5bd009c7c9a9e888077c07535dc0c70aeab242c3 mm: madvise: return correct bytes advised with process_madvise
> > date:   3 months ago
> > compiler: mips-linux-gcc (GCC) 11.3.0
> > reproduce (cppcheck warning):
> >         # apt-get install cppcheck
> >         git checkout 5bd009c7c9a9e888077c07535dc0c70aeab242c3
> >         cppcheck --quiet --enable=style,performance,portability --template=gcc FILE
> > 
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > 
> > cppcheck warnings: (new ones prefixed by >>)
> >>> mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
> >     ret = (total_len - iov_iter_count(&iter)) ? : ret;
> 
> Other way to avoid this warning is by creating another local variable
> that holds the total bytes processed. Having another local variable to
> get rid off some compilation warning doesn't seem proper to me. So,
> leaving this warning unless you ask me to fix this.

Is this a new warning? I do not see it supported by my gcc 10.x. Do we
plan to have it enabled by default? I do not see anything wrong with the
above code and I think this is not an unusual pattern in the kernel.
While you could go with
	if (rotal_len - iov_iter_count(&iter))
		ret = rotal_len - iov_iter_count(&iter);

or do the same with a temporary variable but I am not really sure this would
add to the readability much.
-- 
Michal Hocko
SUSE Labs


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

* Re: [kbuild-all] Re: mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
  2022-06-20 16:29   ` Michal Hocko
@ 2022-06-20 16:54     ` Julia Lawall
  2022-06-20 19:14       ` Michal Hocko
  0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2022-06-20 16:54 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Charan Teja Kalla, kernel test robot, Minchan Kim, kbuild-all,
	linux-kernel, Andrew Morton, Linux Memory Management List



On Mon, 20 Jun 2022, Michal Hocko wrote:

> On Sat 18-06-22 11:25:43, Charan Teja Kalla wrote:
> > Hello Andrew,
> >
> > On 6/18/2022 4:34 AM, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head:   4b35035bcf80ddb47c0112c4fbd84a63a2836a18
> > > commit: 5bd009c7c9a9e888077c07535dc0c70aeab242c3 mm: madvise: return correct bytes advised with process_madvise
> > > date:   3 months ago
> > > compiler: mips-linux-gcc (GCC) 11.3.0
> > > reproduce (cppcheck warning):
> > >         # apt-get install cppcheck
> > >         git checkout 5bd009c7c9a9e888077c07535dc0c70aeab242c3
> > >         cppcheck --quiet --enable=style,performance,portability --template=gcc FILE
> > >
> > > If you fix the issue, kindly add following tag where applicable
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > >
> > > cppcheck warnings: (new ones prefixed by >>)
> > >>> mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
> > >     ret = (total_len - iov_iter_count(&iter)) ? : ret;
> >
> > Other way to avoid this warning is by creating another local variable
> > that holds the total bytes processed. Having another local variable to
> > get rid off some compilation warning doesn't seem proper to me. So,
> > leaving this warning unless you ask me to fix this.
>
> Is this a new warning? I do not see it supported by my gcc 10.x. Do we

cppcheck is a static analysis tool.  It looks like it doesn't have a
proper understanding of ?:

julia

> plan to have it enabled by default? I do not see anything wrong with the
> above code and I think this is not an unusual pattern in the kernel.
> While you could go with
> 	if (rotal_len - iov_iter_count(&iter))
> 		ret = rotal_len - iov_iter_count(&iter);
>
> or do the same with a temporary variable but I am not really sure this would
> add to the readability much.
> --
> Michal Hocko
> SUSE Labs
> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org
>


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

* Re: [kbuild-all] Re: mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
  2022-06-20 16:54     ` [kbuild-all] " Julia Lawall
@ 2022-06-20 19:14       ` Michal Hocko
  2022-06-22  2:37         ` Chen, Rong A
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2022-06-20 19:14 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Charan Teja Kalla, kernel test robot, Minchan Kim, kbuild-all,
	linux-kernel, Andrew Morton, Linux Memory Management List

On Mon 20-06-22 12:54:56, Julia Lawall wrote:
> 
> 
> On Mon, 20 Jun 2022, Michal Hocko wrote:
> 
> > On Sat 18-06-22 11:25:43, Charan Teja Kalla wrote:
> > > Hello Andrew,
> > >
> > > On 6/18/2022 4:34 AM, kernel test robot wrote:
> > > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > > head:   4b35035bcf80ddb47c0112c4fbd84a63a2836a18
> > > > commit: 5bd009c7c9a9e888077c07535dc0c70aeab242c3 mm: madvise: return correct bytes advised with process_madvise
> > > > date:   3 months ago
> > > > compiler: mips-linux-gcc (GCC) 11.3.0
> > > > reproduce (cppcheck warning):
> > > >         # apt-get install cppcheck
> > > >         git checkout 5bd009c7c9a9e888077c07535dc0c70aeab242c3
> > > >         cppcheck --quiet --enable=style,performance,portability --template=gcc FILE
> > > >
> > > > If you fix the issue, kindly add following tag where applicable
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > >
> > > >
> > > > cppcheck warnings: (new ones prefixed by >>)
> > > >>> mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
> > > >     ret = (total_len - iov_iter_count(&iter)) ? : ret;
> > >
> > > Other way to avoid this warning is by creating another local variable
> > > that holds the total bytes processed. Having another local variable to
> > > get rid off some compilation warning doesn't seem proper to me. So,
> > > leaving this warning unless you ask me to fix this.
> >
> > Is this a new warning? I do not see it supported by my gcc 10.x. Do we
> 
> cppcheck is a static analysis tool.  It looks like it doesn't have a
> proper understanding of ?:

Ohh, thanks for the clarification! I thought this was a gcc feature.
Then I would suggest to report a bug report against the static checker
rather than making any changes to the kernel to workaround it.
-- 
Michal Hocko
SUSE Labs


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

* Re: [kbuild-all] Re: mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
  2022-06-20 19:14       ` Michal Hocko
@ 2022-06-22  2:37         ` Chen, Rong A
  0 siblings, 0 replies; 6+ messages in thread
From: Chen, Rong A @ 2022-06-22  2:37 UTC (permalink / raw)
  To: Michal Hocko, Julia Lawall
  Cc: Charan Teja Kalla, kernel test robot, Minchan Kim, kbuild-all,
	linux-kernel, Andrew Morton, Linux Memory Management List



On 6/21/2022 3:14 AM, Michal Hocko wrote:
> On Mon 20-06-22 12:54:56, Julia Lawall wrote:
>>
>>
>> On Mon, 20 Jun 2022, Michal Hocko wrote:
>>
>>> On Sat 18-06-22 11:25:43, Charan Teja Kalla wrote:
>>>> Hello Andrew,
>>>>
[...]
>>>>> cppcheck warnings: (new ones prefixed by >>)
>>>>>>> mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment]
>>>>>      ret = (total_len - iov_iter_count(&iter)) ? : ret;
>>>>
>>>> Other way to avoid this warning is by creating another local variable
>>>> that holds the total bytes processed. Having another local variable to
>>>> get rid off some compilation warning doesn't seem proper to me. So,
>>>> leaving this warning unless you ask me to fix this.
>>>
>>> Is this a new warning? I do not see it supported by my gcc 10.x. Do we
>>
>> cppcheck is a static analysis tool.  It looks like it doesn't have a
>> proper understanding of ?:
> 
> Ohh, thanks for the clarification! I thought this was a gcc feature.
> Then I would suggest to report a bug report against the static checker
> rather than making any changes to the kernel to workaround it.
> 

Hi all,

Sorry for the inconvenience, we have added the warning to ignore list
to avoid reporting it again.

Best Regards,
Rong Chen


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

end of thread, other threads:[~2022-06-22  2:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17 23:04 mm/madvise.c:1438:6: warning: Redundant assignment of 'ret' to itself. [selfAssignment] kernel test robot
2022-06-18  5:55 ` Charan Teja Kalla
2022-06-20 16:29   ` Michal Hocko
2022-06-20 16:54     ` [kbuild-all] " Julia Lawall
2022-06-20 19:14       ` Michal Hocko
2022-06-22  2:37         ` Chen, Rong A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox