[Lldb-commits] [PATCH] D59854: [Host] Add LibraryLoader abstraction around dlopen/LoadLibrary

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In D59854#1443906 <https://reviews.llvm.org/D59854#1443906>, @jingham wrote:

> In D59854#1443904 <https://reviews.llvm.org/D59854#1443904>, @JDevlieghere wrote:
>
> > In D59854#1443891 <https://reviews.llvm.org/D59854#1443891>, @jingham wrote:
> >
> > > It isn't safe to pass "data()" of a StringRef to dlsym.
> >
> >
> > Why?
>
>
> StringRef's aren't guaranteed to be NULL terminated.  Or rather, the Data that gets returned by the data() method is just the raw pointer in the StringRef, which isn't guaranteed to be NULL terminated.


While true, I don't think this is a good reason to make the interface accept a `const char*`, because that means that the interface is exposing an implementation detail.  Instead, the interface should accept a `StringRef`, and the implementation can pass `foo.str().c_str()`.  It might be slightly less efficient (almost to the point of minutia), but this isn't really performance sensitive code, as loading the library will dominate the call to malloc / memcpy.


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

https://reviews.llvm.org/D59854