[PATCH] D73309: [lsan] Factor pthread-specific assumptions out of thread tracking code

Roland McGrath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 24 16:27:12 PST 2020


mcgrathr marked 3 inline comments as done.
mcgrathr added a comment.

I've made the style changes you asked for and clang-format'd the files I touched.  But note that all of these changes are to existing code that I just moved and left exactly as it had been and not anything I'd added.  I erred on the side of leaving things alone rather than cleaning them up.  I've now done the opposite as you asked, though it's not entirely consistent with the style and formatting of the existing code in the same directory.  I'm happy to use whatever style choices you prefer, but it's easiest to do that when the codebase is consistent with those choices already.



================
Comment at: compiler-rt/lib/lsan/lsan_posix.cpp:31
+struct OnStartedArgs {
+  uptr stack_begin, stack_end,
+       cache_begin, cache_end,
----------------
vitalybuka wrote:
> can you make it more consistent?
> type name1;
> type name2;
> ...
> 
Consistent with what?
I'm happy to use whatever style rules you prefer.
But there's no sign that one-declaration-per-line is the prevailing sign in the existing codebase.

I'm changing this as you asked, but it was verbatim moved from the existing code in the other file and all the style choices were not mine.


================
Comment at: compiler-rt/lib/lsan/lsan_posix.cpp:65
+  ThreadContext *context = static_cast<ThreadContext *>(
+      GetThreadRegistryLocked()->FindThreadContextByOsIDLocked(os_id));
+  if (!context) return false;
----------------
vitalybuka wrote:
> is this clang-formated?
It was moved verbatim from the old code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73309





More information about the llvm-commits mailing list