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

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka 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");
+
----------------
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



Repository:
  rL LLVM

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

https://reviews.llvm.org/D55014