[PATCH] D18736: [llvm-c] Improve IR Introspection: Add LLVM{Get, Set}ModuleIdentifier

[Nicole Mazzuca via llvm-commits]

ubsan added inline comments.

================
Comment at: include/llvm-c/Core.h:483-505
@@ -482,4 +482,25 @@
 
 /**
+ * Obtain the identifier of a module. The result must be discarded with
+ * LLVMDisposeMessage.
+ *
+ * @param M Module to obtain identifier of
+ * @param Len Out parameter which holds the length of the returned string.
+ * @return The identifier of M.
+ * @see Module::getModuleIdentifier()
+ */
+char *LLVMGetModuleIdentifier(LLVMModuleRef M, unsigned *Len);
+
+/**
+ * Set the identifier of a module to a string Ident with length Len.
+ *
+ * @param M The module to set identifier
+ * @param Ident The string to set M's identifier to
+ * @param Len Length of Ident
+ * @see Module::setModuleIdentifier()
+ */
+void LLVMSetModuleIdentifier(LLVMModuleRef M, const char *Ident, unsigned Len);
+
+/**
  * Obtain the data layout for a module.
  *
----------------
deadalnix wrote:
> I think you are right, size_t is the correct type here, while I don't think it makes a big difference in practice (I don't expect anyone to get anywhere near 4G of description for a module).
However, everything else uses unsigned for length.

================
Comment at: lib/IR/Core.cpp:161-162
@@ +160,4 @@
+char *LLVMGetModuleIdentifier(LLVMModuleRef M, unsigned *Len) {
+  auto Ret = strdup(unwrap(M)->getModuleIdentifier().c_str());
+  auto ModuleLen = strlen(Ret);
+  assert(ModuleLen < UINT_MAX && "Module identifier is too long");
----------------
deadalnix wrote:
> A string should know its length already, so taking the pointer and then computing strlen is wasteful. It is also not needed to strdup. It is common practice in the C API to return string the caller don't have ownership of. If caller has ownership of something, then a LLVMDisposeXXX method is provided. In this case, I don't think this is necessary.
> 
>   auto &Str = unwrap(M)->getModuleIdentifier()
>   *Length = Str.length();
>   return Str.c_str();
> 
> Also, just take a size_t for length so yu don't need to assert. I see no reason why someone would have such a large identifier that it matters, but, on the other hand, I see no reason to prevent it.
Will do.


http://reviews.llvm.org/D18736