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

MIEngine: Set MajorVersion from the correct place #1441

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

intel-rganesh
Copy link
Contributor

We were previously setting the MajorGdbVersion when the process exited and this was too late.
We need to set this OnStateChanged so we can use this to opt for the latest gdb MI commands.
Additionally include Intel GNU gdb example when fetching the GDB version.

Signed-off-by: intel-rganesh rakesh.ganesh@intel.com

We were previously setting the MajorGdbVersion
when the process exited and this was too late.
We need to set this OnStateChanged so we can
use this to opt for the latest gdb MI commands.

Signed-off-by: intel-rganesh rakesh.ganesh@intel.com
@gregg-miskelly
Copy link
Member

                    _initializationLog.AddLast(line);

Rather than scraping the initialization log after the fact, I think we should just process the line here. Something like:

if (string.IsNullOrEmpty(_gdbVersion))
{
    TryInitializeGdbVersion(line);
}

Refers to: src/MICore/Debugger.cs:961 in 46756e5. [](commit_id = 46756e5, deletion_comment = False)

Copy link
Member

@gregg-miskelly gregg-miskelly left a comment

Choose a reason for hiding this comment

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

See other comments

@WardenGnaw WardenGnaw self-requested a review March 1, 2024 01:56
Try to initialize the gdb version as soon as we add the line to the
initialization log.
Also, add exception handling when setting major gdb version.

Signed-off-by: intel-rganesh rakesh.ganesh@intel.com
Copy link
Member

@gregg-miskelly gregg-miskelly left a comment

Choose a reason for hiding this comment

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

Aside from putting back line 495 now, looks good to me.

Clear initialization log as before.

Signed-off-by: intel-rganesh rakesh.ganesh@intel.com
@WardenGnaw WardenGnaw merged commit 943ab5e into microsoft:main Mar 6, 2024
6 checks passed
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