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

Zachary Turner via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Mar 26 17:25:33 PDT 2019


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





More information about the lldb-commits mailing list