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

Possible bad copy paste in track_process.c #2425

Closed
Bbulatov opened this issue Jun 5, 2024 · 2 comments
Closed

Possible bad copy paste in track_process.c #2425

Bbulatov opened this issue Jun 5, 2024 · 2 comments

Comments

@Bbulatov
Copy link

Bbulatov commented Jun 5, 2024

Hello!
During the static analysis, was found one possible mistake (bad copy past) in code.
Variable tpr->terminate_delay is using and checking in file keepalived/core/track_process.c:905-909. Variable tpr->fork_delay is checking in keepalived/core/track_process.c:895, but in 896 used variable tpr->terminate_delay. Perhaps you should use tpr->fork_delay instead of tpr->terminate_delay in line 896.

изображение

Please clarify whether this needs a fix?

@pqarmitage
Copy link
Collaborator

@Bbulatov Many thanks for reporting this. I think, however, that the problem is worse than you identify, meaning that currently whatever value of the timer is used is irrelevant.

The code at lines 895-897, currently:

                                if (tpr->fork_delay)
                                        tpr->fork_timer_thread = thread_add_timer(master, process_gained_quorum_timer_thread, tpr, tpr->terminate_delay);
                                process_update_track_process_status(tpr, true);

I think should be:

                                if (tpr->fork_delay)
                                        tpr->fork_timer_thread = thread_add_timer(master, process_gained_quorum_timer_thread, tpr, tpr->fork_delay);
                                else
                                        process_update_track_process_status(tpr, true);

In other words, not only is the delay timer the wrong one, as you identified, but process_update_track_process_status() should not be called if there is a fork_delay configured.

I need to think this through carefully and do some testing before producing a patch.

@pqarmitage
Copy link
Collaborator

Commit 3df2947 resolves this issue, and a couple of others I identified while investigating it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants