[PATCH] D55014: Add a new interceptor for getvfsstat(2) from NetBSD

[Kamil Rytarowski via Phabricator via llvm-commits]

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


================
Comment at: test/sanitizer_common/TestCases/NetBSD/getvfsstat.cc:18
+  if ((rv = getvfsstat(NULL, 0, ST_WAIT)) == -1)
+    err(EXIT_FAILURE, "getvfsstat");
+
----------------
vitalybuka wrote:
> krytarowski wrote:
> > vitalybuka wrote:
> > > you can simplify the test with simple asserts here
> > Do you mean: `assert(rv != -1)` ? err(3) is already simple enough so I will keep it.
> I see couple other tiny reasons, which are not strong enough to block the patch, but maybe you'll still consider them.
> 
> The only important lines here are 17 and 25. The rest is just necessary distruction from the test,  so smaller is better.
> 
> Also I'd prefer assert report with line number, which can be used for quick jump to location than current printout of "getvfsstat".
> 
> "if (!buf)" is totally unneeded as it crash anyway, and we don't to that in other tests.
> 
> If you don't CHECK them maybe: printf("Filesystem %d %s %s %s\n", i, buf[i].f_fstypename,   buf[i].f_mntonname, buf[i].f_mntfromname);
> 
> and consistency: compiler_rt tests never use err() in cases like this, mostly asserts
> 
`err(3`) is a BSD style. I will switch it to assert and land soon.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55014