[PATCH] D19731: Parse PDB Name Hash Table

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: lib/DebugInfo/PDB/Raw/NameHashTable.cpp:98
@@ +97,3 @@
+
+  return (Hash * 1664525L + 1013904223L) % 0xFFFFFFFF;
+}
----------------
majnemer wrote:
> Hash is a `uint32_t`, does `% 0xFFFFFFFF` actually do anything?
Honestly I don't think so, but I was wondering about the case where the formula there results in precisely `0xFFFFFFFF`.  Upon closer inspection though, I don't think that's even possible, because `0xFFFFFFFF-1013904223 = 3281063072`, which isn't divisible by `5`.  And 

================
Comment at: lib/DebugInfo/PDB/Raw/NameHashTable.cpp:153
@@ +152,3 @@
+  }
+  return 0xFFFFFFFF;
+}
----------------
majnemer wrote:
> Returning -1 isn't a great interface, who is supposed to call `getIndexForString` ?
I think I can return `0` here instead.  `0` looks to be not a valid name index.  Technically a name index is not actually an index but rather an offset into the names buffer, which you get by indexing the indices array.  offset 0 in the buffer appears to always be a null character.

================
Comment at: lib/DebugInfo/PDB/Raw/NameHashTable.cpp:26
@@ +25,3 @@
+  uint32_t Result = 0;
+  uint32_t Size = Str.size();
+
----------------
majnemer wrote:
> Shouldn't this be a `size_t` ?
Yea probably.


http://reviews.llvm.org/D19731