[PATCH] D78179: [Darwin] Fix symbolization for recent simulator runtimes.

[Dan Liew via Phabricator via llvm-commits]

delcypher marked an inline comment as done.
delcypher added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp:138
   char pid_str_[16];
+  static const char kAtosEnvVar_[sizeof(K_ATOS_ENV_VAR)];
+  // Space for `\0` in `kAtosEnvVar_` is reused for `=`.
----------------
yln wrote:
> I think you can get away without the defining `kAtosEnvVar_` at all:
> Here:
> `char atos_mach_port_env_var_entry_[sizeof(K_ATOS_ENV_VAR) + sizeof(pid_str_)];`
> Above: make it part of the format string:
> `internal_snprintf(.., .., K_ATOS_ENV_VAR "=%s", pid_str);`
> 
> 
@yln
Originally this was a lot cleaner. It was just this at global scope.

```
static const char kAtosEnvVar[] = "__check_mach_ports_lookup";
```

However @kubamracek wanted this sunk into the class itself. This resulted in the mess that's in this current patch. I much preferred how it was before because I didn't need to use macros **at all**.

The approach you've laid out is a definite improvement over what's currently in the patch but it still involves macros.

@kubamracek @yln Can we pick something? Either what @yln is suggesting or my original approach (global variable). I prefer the original approach because **it doesn't involve any macros**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78179