[PATCH] D21503: [llvm-dwarfdump] - Teach dwarfdump to dump gdb-index section.

[George Rimar via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D21503#550217, @dblaikie wrote:

> Looks alright - some basic cleanup to do before committing.


Thanks ! I'll commit soon.

> (we might consider the format changing over time to make it a bit more terse/legible about what it's meant to be describing rather than the mechanical format that it's using to represent that (eg: dumping the string offsets might not be generically useful, just printing the strings might suffice))


I think if format will change, we should add the testcases, but not remove/modify old ones. I think that offsets dumping is not just usefull but it is the main aim of that testcase because it is the only information we extract from binary, when the string values is what we calculate by ourselves in this dumper. So I would prefer not to print the strings values rather than ignoring offsets dumping, but I think it is useful to look at them either, that works like a proof for fact that we dumped everything correctly.


================
Comment at: lib/DebugInfo/DWARF/DWARFGdbIndex.cpp:114
@@ +113,3 @@
+  CuList.reserve(CuListSize);
+  for (uint32_t I = 0; I < CuListSize; ++I) {
+    uint64_t CuOffset = Data.getU64(&Offset);
----------------
dblaikie wrote:
> Prefer 'i' (lowercase) for loop indexes
Hmm, I`ll do that change, but it looks to be violation of LLVM codestyle for me.


https://reviews.llvm.org/D21503