[cfe-commits] r84198 - in /cfe/trunk: include/clang-c/Index.h tools/CIndex/CIndex.cpp tools/CIndex/CIndex.exports

[Daniel Dunbar]

Hi Steve,

I just noticed another problem here...

On Thu, Oct 15, 2009 at 1:04 PM, Steve Naroff <snaroff at apple.com> wrote:
> Author: snaroff
> Date: Thu Oct 15 15:04:39 2009
> New Revision: 84198
>
> URL: http://llvm.org/viewvc/llvm-project?rev=84198&view=rev
> Log:
> Implement <rdar://problem/7303432> [Clang/Index] In-memory-style AST generation API (initial API implementation).
>
> Added clang_createTranslationUnitFromSourceFile().
> Changed clang_createIndex() to lookup the location of clang (using dladdr).
>
> Modified:
>    cfe/trunk/include/clang-c/Index.h
>    cfe/trunk/tools/CIndex/CIndex.cpp
>    cfe/trunk/tools/CIndex/CIndex.exports
>
> ==============================================================================
> --- cfe/trunk/tools/CIndex/CIndex.cpp (original)
> +++ cfe/trunk/tools/CIndex/CIndex.cpp Thu Oct 15 15:04:39 2009
...
> +static const char *clangPath;
> +
>  CXIndex clang_createIndex()
>  {
>   // FIXME: Program is leaked.
> +
> +  // Find the location where this library lives (libCIndex.dylib).
> +  // We do the lookup here to avoid poking dladdr too many times.
> +  // This silly cast below avoids a C++ warning.
> +  Dl_info info;
> +  if (dladdr((void *)(uintptr_t)clang_createTranslationUnit, &info) == 0)
> +    assert(0 && "Call to dladdr() failed");
> +
> +  llvm::sys::Path CIndexPath(info.dli_fname);
> +  std::string CIndexDir = CIndexPath.getDirname();
> +
> +  // We now have the CIndex directory, locate clang relative to it.
> +  std::string ClangPath = CIndexDir + "/../bin/clang";
> +
> +  clangPath = ClangPath.c_str();

This isn't safe, its storing a dangling pointer into clangPath. Also,
it would be good not to add a global variable.

I think it would be better to have this live in CXIndex. While at it,
it makes sense to lazily compute it, for clients that don't use
clang_createTranslationUnitFromSourceFile.

 - Daniel