[PATCH] D124212: [sanitizer] Use canonical syscalls everywhere

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 12 13:08:55 PDT 2022


jrtc27 added a comment.

In D124212#3509972 <https://reviews.llvm.org/D124212#3509972>, @eugenis wrote:

> In D124212#3509846 <https://reviews.llvm.org/D124212#3509846>, @glaubitz wrote:
>
>> In D124212#3509781 <https://reviews.llvm.org/D124212#3509781>, @jrtc27 wrote:
>>
>>> There's a lot of really quite wonky whitespace reformatting in this diff
>>
>> FWIW, I have already a draft for fixing this. Will create a revision tomorrow.
>>
>> EDIT: I mean the build failure on sparc64 ;).
>
> Thanks. Sorry for the breakage. It appears that sparc64 is the only 64-bit target without newfstatat syscall.

That's not true, alpha and hppa64 are the same from what I can tell, maybe others too, I stopped looking after that point, but sparc64 may be the only architecture compiler-rt supports that's like that.

> I've considered keeping the non-canonical-syscalls code path for cases like this, but getting rid of all the preprocessor conditionals was too tempting :)
>
> We should re-lint this entire file at some point, but there is never a good time for that - you don't want to mix it with a functional change, and then it also makes it harder to revert changes so you'd want to wait a while after any non-trivial commit... Maybe after you land the sparc fix.

Sure, but you should never be changing the whitespace for `#else` without also changing `#if` to match (or vice-versa, or ditto with `#endif`), currently you have mismatched indentation all over the place which is what I meant by wonky, it's extremely misleading and hard to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124212



More information about the llvm-commits mailing list