Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kernel: Poll: Code Suspected Logic Problem #31282

Closed
Zhaoningx opened this issue Jan 13, 2021 · 0 comments · Fixed by #31283
Closed

Kernel: Poll: Code Suspected Logic Problem #31282

Zhaoningx opened this issue Jan 13, 2021 · 0 comments · Fixed by #31283
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@Zhaoningx
Copy link
Collaborator

I found that there is a bug in the poll source code, the code as below:
static inline int register_events()
{
int events_registered = 0;

for (int ii = 0; ii < num_events; ii++) {
	k_spinlock_key_t key;
	uint32_t state;

	key = k_spin_lock(&lock);
	if (is_condition_met(&events[ii], &state)) {
		set_event_ready(&events[ii], state);
		poller->is_polling = false;
	} else if (!just_check && poller->is_polling) {
		int rc = register_event(&events[ii], poller);
		**if (rc == 0) {
			events_registered += 1;
		} else {
			__ASSERT(false, "unexpected return code\n");
		}**
	}
	k_spin_unlock(&lock, key);
}

return events_registered;

}

register_event function code as below:
static inline int register_event()
{
switch (event->type) {
case K_POLL_TYPE_SEM_AVAILABLE:
__ASSERT(event->sem != NULL, "invalid semaphore\n");
add_event(&event->sem->poll_events, event, poller);
break;
case K_POLL_TYPE_DATA_AVAILABLE:
__ASSERT(event->queue != NULL, "invalid queue\n");
add_event(&event->queue->poll_events, event, poller);
break;
case K_POLL_TYPE_SIGNAL:
__ASSERT(event->signal != NULL, "invalid poll signal\n");
add_event(&event->signal->poll_events, event, poller);
break;
case K_POLL_TYPE_IGNORE:
/* nothing to do */
break;
default:
__ASSERT(false, "invalid event type\n");
break;
}

event->poller = poller;

**return 0;**

}

As you can see, rc is the return value of the function register_event. The judgment condition is whether rc is equal to 0. Analysis of the function register_event code shows that the function has only one return value 0, so if the function runs without an ASSERT error, the return value must be 0, so I think logic of this code is a bug and needs to be fixed.

@Zhaoningx Zhaoningx added the bug The issue is a bug, or the PR is fixing a bug label Jan 13, 2021
@Zhaoningx Zhaoningx self-assigned this Jan 13, 2021
Zhaoningx added a commit to Zhaoningx/zephyr that referenced this issue Jan 18, 2021
register_event always returns 0, so the conditional will
always take the first branch and code in the else part
is never reached.

Fixes zephyrproject-rtos#31282

Signed-off-by: Ningx Zhao <ningx.zhao@intel.com>
@nashif nashif added the priority: low Low impact/importance bug label Jan 18, 2021
nashif pushed a commit that referenced this issue Jan 18, 2021
register_event always returns 0, so the conditional will
always take the first branch and code in the else part
is never reached.

Fixes #31282

Signed-off-by: Ningx Zhao <ningx.zhao@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants