[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:34
+  FileWriter(std::ostream &S, llvm::support::endianness B) : OS(S), ByteOrder(B) {}
+  ~FileWriter();
+  bool writeU8(uint8_t Value);
----------------
clayborg wrote:
> aprantl wrote:
> > This whole interface needs Doxygen comments.
> Agreed, but I am not going to do it before we agree on the API.
> 
> I just didn't know if this class was going to be thrown out in favor of the MC layer. I couldn't figure out how to use it for raw files so I made this class, but I figured someone with more llvm expertise might let me know how to do it or how to do it another way.
> 
> I figured FileWriter would need changes so I just want to iterate on this and get to the "lgtm once we have doxygen comments", then I will add full docs. Is that ok? 
Looks like the comments are still missing.


================
Comment at: include/llvm/DebugInfo/GSYM/FileWriter.h:43
+  bool writeData(llvm::ArrayRef<uint8_t> Data);
+  bool writeCStr(const char *CStr);
+  bool fixup32(uint32_t Value, off_t Offset);
----------------
clayborg wrote:
> aprantl wrote:
> > clayborg wrote:
> > > It needs both the ability to write raw data (for raw data after being swapped and for internal implementation) and NULL terminated C strings (for string table).
> > Now that you changed the other API to take an ArrayRef, there should be no problem changing this method to:
> > 
> > bool writeNullTerminated(StringRef s);
> > 
> > ?
> I can just switch this to StringRef and have the user be on the hook for not passing in a StringRef with embedded NULL characters. We can always write the entire string and always add a NULL. Is that ok?
Ideally, then the method should be called writeNullTerminated or something to that end.


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

https://reviews.llvm.org/D63828