[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:42
+  bool writeData(const void *Src, size_t SrcLen);
+  bool writeCStr(const char *CStr);
+  bool fixup32(uint32_t Value, off_t Offset);
----------------
clayborg wrote:
> aprantl wrote:
> > This probably shouldn't exist :-)
> I believe it should. It clearly states we are writing a NULL terminated C string. 
> 
> If we pass a StringRef the simple cases are clear:
> - StringRef("hello"): write out "hello\0"
> - StringRef("")? Write out "\0"
> 
> But there are many things that are not as clear if we pass in:
> - StringRef()? Write out nothing since it has no data? Or write out "\0"?
> - StringRef("hello\0world\0")? Just write out "hello\0" since that is the first C string?
> - StringRef("hello\0") (where size includes the NULL termination byte) write out "hello\0". We will need to check if the string ends with '\0', or contains any '\0' for so we avoid emitting an extra NULL character
> 
> 
> 
Let's ask the underlying question: Which of the above use-cases does a GSYM writer need, and why isn't the StringRef case sufficient?


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

https://reviews.llvm.org/D63828