[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