[PATCH] D63828: Add FileWriter to GSYM and encode/decode functions to AddressRange and AddressRanges

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/DebugInfo/GSYM/FileWriter.h:41
+  bool writeUnsigned(uint64_t Value, size_t N);
+  bool writeData(const void *Src, size_t SrcLen);
+  bool writeCStr(const char *CStr);
----------------
This should take a StringRef


================
Comment at: include/llvm/DebugInfo/GSYM/FileWriter.h:42
+  bool writeData(const void *Src, size_t SrcLen);
+  bool writeCStr(const char *CStr);
+  bool fixup32(uint32_t Value, off_t Offset);
----------------
This probably shouldn't exist :-)


================
Comment at: lib/DebugInfo/GSYM/FileWriter.cpp:14
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
----------------
Why do you need this? Is there no abstraction in support for what you need?


================
Comment at: lib/DebugInfo/GSYM/FileWriter.cpp:76
+
+bool FileWriter::writeCStr(const char *CStr) {
+  assert(CStr != nullptr);
----------------
This function looks like it's just asking for trouble :-)


================
Comment at: lib/DebugInfo/GSYM/Range.cpp:102
+  Ranges.resize(NumRanges);
+  for (size_t I = 0; I < NumRanges; ++I)
+    Ranges[I].decode(Data, BaseAddr, Offset);
----------------
range-based for?


================
Comment at: unittests/DebugInfo/GSYM/GSYMTest.cpp:419
+  FW.writeCStr(Hello);
+  // Test Seek, Tell using Fixup32
+  FW.fixup32(U32, FixupOffset);
----------------
.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63828/new/

https://reviews.llvm.org/D63828