[PATCH] D143418: [libclang] Add API to set preferred temp dir path

Igor Kushnir via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 10 22:21:59 PST 2023


vedgy added a comment.

In D143418#4118905 <https://reviews.llvm.org/D143418#4118905>, @aaron.ballman wrote:

> I don't think "tries hard" is a good enough guarantee for where files are placed. I'm not comfortable with the security posture of that approach as it could potentially leak sensitive information (try to override the temp directory to something that's chroot jailed, get sensitive files showing up in the system temp directory anyway).

That's why the function's documentation explicitly doesn't guarantee anything. It should be safe to assume that security-sensitive users would at least read the documentation. If this function and potential future function like it are named specifically, a responsible security use case wouldn't be better off. The only safety advantage would be to those who don't bother reading the documentation. But why should we care much about such hypothetical careless security needs?

>> Does the multithreading issue mean that `clang_parseTranslationUnit_Impl()` can be called in a non-main thread and thus concurrently with `clang_CXIndex_setPreferredTempDirPath()`?
>
> Yes. However, I don't think that situation is officially supported (it's more that we don't want to knowingly make it harder to support in the future).

All right. Let's do it via a new constructor then. Unfortunately, supporting different `CXIndexOptions` struct sizes/versions would complicate the libclang implementation and the libclang user code. But the alternative of adding a new constructor function each time can either grow to a large number of function parameters unneeded by most users or to an exponential number of overloads that support different usage requirements.

How about including existing options, which //should// be set in constructor, in the initial struct version and deprecating the corresponding setters?

  typedef struct CXIndexOptions {
    uint32_t size; // sizeof(struct CIndexOptions), used for option versioning
    unsigned globalOptions;
    const char *preferredTempDirPath;
    const char *invocationEmissionPath;
  } CXIndexOptions;

The naming of struct data members is inconsistent in libclang's Index.h. They start with a lower-case letter in about half of the structs. Which naming scheme should the members of the new struct use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418



More information about the cfe-commits mailing list