[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

Mon Dec 31 06:24:50 PST 2018

krytarowski added inline comments.

================
Comment at: lib/esan/esan_interceptors.cpp:90
   } while (false)
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file)                                \
+  do {                                                                         \
----------------
dvyukov wrote:
> The idea behind defining a no-op version in sanitizer_common_interceptors.inc was exactly that tools that are not interested in it does not need to be changed. So please remove this.
I would add an intermediate revision to drop redefinitions in this file.

================
Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259

+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \
+  if (file) {                                   \
----------------
dvyukov wrote:
> An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and then do:
> 
>   if (path)
>     Acquire(thr, pc, File2addr(path))
> 
> Just to not multiply entities. But neither approach looks strictly better to me, so I am not too strong about it.
Is `File2addr()` producing any useful result?

Reusing `COMMON_INTERCEPTOR_FILE_OPEN` looks fine.

Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157