Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add GetProcessEnvironment command to diagnostics server #40556
Add GetProcessEnvironment command to diagnostics server #40556
Changes from 10 commits
a8674e5
5303432
945bdfc
eca8062
8a64ed1
e2c4be2
dbf5055
8e3e93e
102f207
32e51d4
970ef47
d0faccc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
In the overflow case this still writes lots of data even though GetEnvironmentBlockSize() specified only 4 bytes would be written. Separately I would assume in the overflow case we'd want to return an error in the header and write 0 bytes.
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.
The logic should leave
_nEnvEntries
set to0
, so the payload would end up saying the continuation has 4 bytes and those 4 bytes should be the number 0 since there won't be any entries in the array. I could modify it so that the continuation length in the message payload is 0 instead and let the user treat that as an error. Would that be preferable?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.
Is there a companion PR to the diagnostics repo with the spec update?
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.
There will be. It hasn't been created yet.
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.
We don't really care how the JIT optimizes this right?
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.
I don't think so. This was held over from doing a copy/paste of the proj file for another event pipe test. I'll remove.