[PATCH] D44036: OpenBSD UBsan support / common part 3

Vitaly Buka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 2 14:49:10 PST 2018


vitalybuka added a comment.

In https://reviews.llvm.org/D44036#1026053, @devnexen wrote:

> In https://reviews.llvm.org/D44036#1026030, @vitalybuka wrote:
>
> > Title of the patch is not useful, but it hard to make it as this patch is still too large to my taste.
> >  E.g. There is no reason to have ReadProcMaps and GetTls changes in a single patch.
> >
> > Ideally:
> >  One patch for trivial "SANITIZER_OPENBSD ||"
> >  For the rest. One patch per fixed function.
>
>
> Ok but the GetTid change relies a bit on my other patch


Use your best judgment. Make sure that if when you spit something parts are not harder to understand that original patch.
Sometimes even for large but trivial patch, first round comments can make it hard to continue review.

Also from LLVM dev policy:

> Split your patch into multiple smaller patches that build on each other. The smaller your patch, the higher the probability that somebody will take a quick look at it.




Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D44036





More information about the llvm-commits mailing list