-
Notifications
You must be signed in to change notification settings - Fork 217
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
Added parsing support for the QNX platform Coredump's thread ids format and fixed VSCODE .devcontainer compilation failure #1398
base: main
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree |
@@ -44,7 +44,7 @@ internal class ThreadCache | |||
private List<DebuggedThread> _deadThreads; | |||
private List<DebuggedThread> _newThreads; | |||
private Dictionary<string, List<int>> _threadGroups; | |||
private static uint s_targetId = uint.MaxValue; | |||
private static uint s_targetId = 0; // Thread ids should be start at 0 in the default scenario |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you talk about why you changed this part? Unfortunately there are no comments to say for sure, but I think the idea of starting with uint.MaxValue
was to select something that was highly unlikely to clash with actual thread ids in the case that the thread id couldn't be parsed.
This code was added in commit 12632f3280da94787c009c8a97accbe797ac6994, but the author didn't add any more details, and I am sure the author no longer remembers.
{ | ||
// In QNX gdb coredumps the thread name is in the form:"pid <pid> tid <pid>", eg "pid 4194373 tid 308" | ||
int tid_pos = targetId.IndexOf("tid ", StringComparison.Ordinal); | ||
int len = targetId.Length - (tid_pos + 4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to put the rest of this block in if (tid_pos >= 0)
// In QNX gdb coredumps the thread name is in the form:"pid <pid> tid <pid>", eg "pid 4194373 tid 308" | ||
int tid_pos = targetId.IndexOf("tid ", StringComparison.Ordinal); | ||
int len = targetId.Length - (tid_pos + 4); | ||
if (len > 0 && System.UInt32.TryParse(targetId.Substring(tid_pos + 4, len), out tid) && tid != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Use a constant for tid
and use '<const_name>.Length` instead of '4' so it is obvious what the '4' means.
Example:
const string tidPrefix = "tid ";
int tid_pos = targetId.IndexOf(tidPrefix, ...)
int len = targetId.Lenth - (tid_pos + tidPrefix.Length);
if (... && UInt32.TryParse(targetId.Substring(tid_pos + tidPrefix.Length...)
"image":"mcr.microsoft.com/vscode/devcontainers/dotnet:6.0-focal", | ||
"remoteUser": "root" | ||
// "postCreateCommand": "", | ||
// "build": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just delete the commented out code
This PR fixes two issues.