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

Revert "break from long loops on shutdown request" #1006

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

en-sc
Copy link
Collaborator

@en-sc en-sc commented Jan 26, 2024

This reverts commits 2e920a2 and 8dbb125.

The reason is, after openocd_is_shutdown_pending() becomes true, arbitrary command may be executed:

  • In target_destroy() and the corresponding target->type->deinit_target().
  • In user-specifyed pre_shutdown_commands list.

Change-Id: Icd00d1d954cd45e255880a6f76c3a74c098d6a17

@en-sc
Copy link
Collaborator Author

en-sc commented Jan 26, 2024

This addresses #1005.

This reverts commits 2e920a2 and
8dbb125.

The reason is, after `openocd_is_shutdown_pending()` becomes true,
arbitrary command may be executed:
* In `target_destroy()` and the corresponding
  `target->type->deinit_target()`.
* In user-specifyed `pre_shutdown_commands` list.

Change-Id: Icd00d1d954cd45e255880a6f76c3a74c098d6a17
Signed-off-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com>
@JanMatCodasip
Copy link
Collaborator

Thank you for preparing the revert. I'd recommend to merge this on Monday if there is no other feedback by then.

Copy link
Collaborator

@MarekVCodasip MarekVCodasip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thank for preparing the revert.

@JanMatCodasip JanMatCodasip merged commit 87331a8 into riscv-collab:riscv Jan 29, 2024
4 checks passed
@en-sc en-sc deleted the en-sc/break-ll-revert branch August 14, 2024 18:39
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

Successfully merging this pull request may close these issues.

3 participants