R2RDump - Output in XML format #18425
Conversation
acmyu
changed the title
acmyu
force-pushed
the
xml
branch
2 times, most recently
from
6ed924e to
a6fb524
Compare
acmyu
force-pushed
the
xml
branch
2 times, most recently
from
64b4c9a to
470f755
Compare
acmyu
changed the title
| { | ||
| public uint StartOffset { get; } | ||
| public uint StopOffset { get; } | ||
| public uint StartOffset { get; set; } |
There was a problem hiding this comment.
Why did you make all the properties publicly writeable? I thought the idea was they get set as we read each data structure from the image and from then on these should all be immutable.
There was a problem hiding this comment.
The XmlSerializer required the properties to have a public setter. When it didn't have one, it didn't serialize those properties
src/tools/r2rdump/R2RDump.cs
Outdated
| NativeParser availableTypesParser = new NativeParser(r2r.Image, availableTypesSectionOffset); | ||
| NativeHashtable availableTypes = new NativeHashtable(r2r.Image, availableTypesParser, (uint)(availableTypesSectionOffset + section.Size)); | ||
| _writer.WriteLine(availableTypes.ToString()); | ||
| if(!_xml) |
There was a problem hiding this comment.
The split we have here where we have if(_xml) all over tells me we should refactor into a TextDumper / XmlDumper class. It would be nice if we could just create the right text / xml dumper class when we're parsing the input arguments and save all these ifs. In the future, if we added, say, a JSON emitter mode, it would be a matter of implementing a new set of overrides instead of having to look through all the ifs.
|
@dotnet-bot Test this please |
src/tools/r2rdump/XmlDumper.cs
Outdated
| } | ||
| break; | ||
| case R2RSection.SectionType.READYTORUN_SECTION_COMPILER_IDENTIFIER: | ||
| AddXMLNode("CompileIdentifier", _r2r.CompileIdentifier, contentsNode); |
There was a problem hiding this comment.
Should this be "CompilerIdentifier"?
src/tools/r2rdump/XmlDumper.cs
Outdated
| return; | ||
| } | ||
|
|
||
| _writer.Write(" "); |
There was a problem hiding this comment.
Do you ever expect to hit this code down here? It seems like if a parent node wasn't specified, you wouldn't want to write anything.
|
LGTM. I left a couple of comments. |
nattress
left a comment
nattress
left a comment
There was a problem hiding this comment.
Looks great - thanks for splitting out the two dumpers!
compilerIdentifier typo
R2RDump - Output in XML format Commit migrated from dotnet/coreclr@8684550
4 participants
No description provided.