diff mbox series

[RFC,v2,08/13] Fix: perform lookup then test for condition

Message ID 20170918225206.17725-9-jonathan.rajotte-julien@efficios.com
State Superseded, archived
Delegated to: Jérémie Galarneau
Headers show
Series Sessiond teardown overhaul | expand

Commit Message

Jonathan Rajotte Sept. 18, 2017, 10:52 p.m. UTC
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Jérémie Galarneau Dec. 14, 2017, 1:56 a.m. UTC | #1
Please add a description of what this fixes, not just how it fixes it.

In this case, I see that the error path is triggered anytime
the pollfd does not match the first app socket in the wait
queue.

On 18 September 2017 at 18:52, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/main.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index 3596d7e3..8cffa6f6 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -1899,8 +1899,11 @@ static void sanitize_wait_queue(struct ust_reg_wait_queue *wait_queue)
>
>                 cds_list_for_each_entry_safe(wait_node, tmp_wait_node,
>                                 &wait_queue->head, head) {
> -                       if (pollfd == wait_node->app->sock &&
> -                                       (revents & (LPOLLHUP | LPOLLERR))) {
> +                       if (pollfd != wait_node->app->sock) {
> +                               continue;
> +                       }
> +
> +                       if (revents & (LPOLLHUP | LPOLLERR)) {

The fix is correct, but this check can be performed before this loop.

Since (LPOLLHUP | LPOLLERR) are the only expected poll events
at this point, we can log the "Unexpected poll events" message
and goto error before entering the wait queue loop, which
will simplify its body.

Jérémie

>                                 cds_list_del(&wait_node->head);
>                                 wait_queue->count--;
>                                 ust_app_destroy(wait_node->app);
> --
> 2.11.0
>
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 3596d7e3..8cffa6f6 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -1899,8 +1899,11 @@  static void sanitize_wait_queue(struct ust_reg_wait_queue *wait_queue)
 
 		cds_list_for_each_entry_safe(wait_node, tmp_wait_node,
 				&wait_queue->head, head) {
-			if (pollfd == wait_node->app->sock &&
-					(revents & (LPOLLHUP | LPOLLERR))) {
+			if (pollfd != wait_node->app->sock) {
+				continue;
+			}
+
+			if (revents & (LPOLLHUP | LPOLLERR)) {
 				cds_list_del(&wait_node->head);
 				wait_queue->count--;
 				ust_app_destroy(wait_node->app);