-
-
Notifications
You must be signed in to change notification settings - Fork 223
Conversation
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.
Thanks for your contribution. I found a few accuracy issues to please clean up before we merge.
/// <summary> | ||
/// The base address of the PEB structure in the process memory. | ||
/// </summary> | ||
public IntPtr PebBaseAddress; |
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.
Since changing this from IntPtr
to PEB*
later will be a binary breaking change, can we do so now? This will involve defining the PEB
structure. Let me know if you have any trouble doing that.
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 format of the PEB
structure is different for 32-bit windows and 64-bit Windows. How do you usually handle such structs?
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.
Eww.. We know how to handle pointer size differences but that looks like a totally different layout. Well, we could define both and leave folks to cast the pointer to the right one, or we could define a ref assembly that only has the common fields and two per-architecture runtime assemblies to implement the right one for each. Both are a bit icky. The first is easier while the second one is probably the best world.
I don't have time for the latter option just now, so if you can figure it out, great. Otherwise the first option may be what we must go with.
I guess since the start out similarly, another option is to only define the first part as far as the two are common.
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 left it as void* at the moment; hope that works for you.
@AArnott, thanks for the feedback. I addressed some of your comments in the code & left some comments for the others. |
@qmfrederik How do you feel about my squashing your PRs on completion, given they are accumulating a lot of 'fix up' commits? |
Sure, go for it! |
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.
Looks good. I guess the test you added just needs to be updated.
No description provided.