linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [vfs:work.epoll 17/27] fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing
@ 2020-10-26  2:09 kernel test robot
  2020-10-26  2:35 ` Al Viro
  0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2020-10-26  2:09 UTC (permalink / raw)
  To: Al Viro; +Cc: kbuild-all, linux-fsdevel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git work.epoll
head:   319c15174757aaedacc89a6e55c965416f130e64
commit: ff07952aeda8563d5080da3a0754db83ed0650f6 [17/27] ep_send_events_proc(): fold into the caller
compiler: h8300-linux-gcc (GCC) 9.3.0

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


"cppcheck warnings: (new ones prefixed by >>)"
>> fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
     events++;
     ^

vim +1629 fs/eventpoll.c

  1565	
  1566	static int ep_send_events(struct eventpoll *ep,
  1567				  struct epoll_event __user *events, int maxevents)
  1568	{
  1569		struct epitem *epi, *tmp;
  1570		LIST_HEAD(txlist);
  1571		poll_table pt;
  1572		int res = 0;
  1573	
  1574		init_poll_funcptr(&pt, NULL);
  1575	
  1576		ep_start_scan(ep, 0, false, &txlist);
  1577	
  1578		/*
  1579		 * We can loop without lock because we are passed a task private list.
  1580		 * Items cannot vanish during the loop because ep_scan_ready_list() is
  1581		 * holding "mtx" during this call.
  1582		 */
  1583		lockdep_assert_held(&ep->mtx);
  1584	
  1585		list_for_each_entry_safe(epi, tmp, &txlist, rdllink) {
  1586			struct wakeup_source *ws;
  1587			__poll_t revents;
  1588	
  1589			if (res >= maxevents)
  1590				break;
  1591	
  1592			/*
  1593			 * Activate ep->ws before deactivating epi->ws to prevent
  1594			 * triggering auto-suspend here (in case we reactive epi->ws
  1595			 * below).
  1596			 *
  1597			 * This could be rearranged to delay the deactivation of epi->ws
  1598			 * instead, but then epi->ws would temporarily be out of sync
  1599			 * with ep_is_linked().
  1600			 */
  1601			ws = ep_wakeup_source(epi);
  1602			if (ws) {
  1603				if (ws->active)
  1604					__pm_stay_awake(ep->ws);
  1605				__pm_relax(ws);
  1606			}
  1607	
  1608			list_del_init(&epi->rdllink);
  1609	
  1610			/*
  1611			 * If the event mask intersect the caller-requested one,
  1612			 * deliver the event to userspace. Again, ep_scan_ready_list()
  1613			 * is holding ep->mtx, so no operations coming from userspace
  1614			 * can change the item.
  1615			 */
  1616			revents = ep_item_poll(epi, &pt, 1);
  1617			if (!revents)
  1618				continue;
  1619	
  1620			if (__put_user(revents, &events->events) ||
  1621			    __put_user(epi->event.data, &events->data)) {
  1622				list_add(&epi->rdllink, &txlist);
  1623				ep_pm_stay_awake(epi);
  1624				if (!res)
  1625					res = -EFAULT;
  1626				break;
  1627			}
  1628			res++;
> 1629			events++;
  1630			if (epi->event.events & EPOLLONESHOT)
  1631				epi->event.events &= EP_PRIVATE_BITS;
  1632			else if (!(epi->event.events & EPOLLET)) {
  1633				/*
  1634				 * If this file has been added with Level
  1635				 * Trigger mode, we need to insert back inside
  1636				 * the ready list, so that the next call to
  1637				 * epoll_wait() will check again the events
  1638				 * availability. At this point, no one can insert
  1639				 * into ep->rdllist besides us. The epoll_ctl()
  1640				 * callers are locked out by
  1641				 * ep_scan_ready_list() holding "mtx" and the
  1642				 * poll callback will queue them in ep->ovflist.
  1643				 */
  1644				list_add_tail(&epi->rdllink, &ep->rdllist);
  1645				ep_pm_stay_awake(epi);
  1646			}
  1647		}
  1648		ep_done_scan(ep, 0, false, &txlist);
  1649	
  1650		return res;
  1651	}
  1652	

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

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

* Re: [vfs:work.epoll 17/27] fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing
  2020-10-26  2:09 [vfs:work.epoll 17/27] fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing kernel test robot
@ 2020-10-26  2:35 ` Al Viro
  2020-10-26  9:45   ` [kbuild-all] " Rong Chen
  0 siblings, 1 reply; 3+ messages in thread
From: Al Viro @ 2020-10-26  2:35 UTC (permalink / raw)
  To: kernel test robot; +Cc: kbuild-all, linux-fsdevel

On Mon, Oct 26, 2020 at 10:09:47AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git work.epoll
> head:   319c15174757aaedacc89a6e55c965416f130e64
> commit: ff07952aeda8563d5080da3a0754db83ed0650f6 [17/27] ep_send_events_proc(): fold into the caller
> compiler: h8300-linux-gcc (GCC) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> "cppcheck warnings: (new ones prefixed by >>)"
> >> fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
>      events++;
>      ^

Who the hell has come up with that warning?  What happens is,
essentially,

f(..., events, ....)
	loop in which we have
		g(events, something); // store the next sample
		events++;

More specifically, it's 
>   1620			if (__put_user(revents, &events->events) ||
>   1621			    __put_user(epi->event.data, &events->data)) {
>   1622				list_add(&epi->rdllink, &txlist);
>   1623				ep_pm_stay_awake(epi);
>   1624				if (!res)
>   1625					res = -EFAULT;
>   1626				break;
>   1627			}
>   1628			res++;
> > 1629			events++;

If anything, that should be reported to the maintainers of the buggy code.
Which is not the kernel in this case.

Google search on that thing brings this:

	Cppcheck is an analysis tool for C/C++ code. It detects the types of
bugs that the compilers normally fail to detect. The goal is no false positives.

IOW, that should be reported to the authors of that thing, seeing that
their stated goal is obviously missed in this case.  Badly.  Assignments of
function parameters can be perfectly idiomatic and this case is such.

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

* Re: [kbuild-all] Re: [vfs:work.epoll 17/27] fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing
  2020-10-26  2:35 ` Al Viro
@ 2020-10-26  9:45   ` Rong Chen
  0 siblings, 0 replies; 3+ messages in thread
From: Rong Chen @ 2020-10-26  9:45 UTC (permalink / raw)
  To: Al Viro, kernel test robot; +Cc: kbuild-all, linux-fsdevel



On 10/26/20 10:35 AM, Al Viro wrote:
> On Mon, Oct 26, 2020 at 10:09:47AM +0800, kernel test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git work.epoll
>> head:   319c15174757aaedacc89a6e55c965416f130e64
>> commit: ff07952aeda8563d5080da3a0754db83ed0650f6 [17/27] ep_send_events_proc(): fold into the caller
>> compiler: h8300-linux-gcc (GCC) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>>
>> "cppcheck warnings: (new ones prefixed by >>)"
>>>> fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
>>       events++;
>>       ^
> Who the hell has come up with that warning?  What happens is,
> essentially,
>
> f(..., events, ....)
> 	loop in which we have
> 		g(events, something); // store the next sample
> 		events++;
>
> More specifically, it's
>>    1620			if (__put_user(revents, &events->events) ||
>>    1621			    __put_user(epi->event.data, &events->data)) {
>>    1622				list_add(&epi->rdllink, &txlist);
>>    1623				ep_pm_stay_awake(epi);
>>    1624				if (!res)
>>    1625					res = -EFAULT;
>>    1626				break;
>>    1627			}
>>    1628			res++;
>>> 1629			events++;
> If anything, that should be reported to the maintainers of the buggy code.
> Which is not the kernel in this case.
>
> Google search on that thing brings this:
>
> 	Cppcheck is an analysis tool for C/C++ code. It detects the types of
> bugs that the compilers normally fail to detect. The goal is no false positives.
>
> IOW, that should be reported to the authors of that thing, seeing that
> their stated goal is obviously missed in this case.  Badly.  Assignments of
> function parameters can be perfectly idiomatic and this case is such.

Hi Al,

Thanks for the explanation, we'll avoid sending out such reports 
automatically
and double check the warning.

Best Regards,
Rong Chen



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

end of thread, other threads:[~2020-10-26 10:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26  2:09 [vfs:work.epoll 17/27] fs/eventpoll.c:1629:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing kernel test robot
2020-10-26  2:35 ` Al Viro
2020-10-26  9:45   ` [kbuild-all] " Rong Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).