Closed mslusarz closed 7 years ago
Merging #431 into master will increase coverage by
2.84%
. The diff coverage is80.76%
.
@@ Coverage Diff @@
## master #431 +/- ##
==========================================
+ Coverage 75.95% 78.79% +2.84%
==========================================
Files 70 70
Lines 8815 8796 -19
Branches 1781 1772 -9
==========================================
+ Hits 6695 6931 +236
+ Misses 1628 1424 -204
+ Partials 492 441 -51
Flag | Coverage Δ | |
---|---|---|
#ltp_tests | 45.15% <38.46%> (+0.4%) |
:arrow_up: |
#sqlite_tests | 45.13% <50%> (+0.26%) |
:arrow_up: |
#tests_antool | 15.08% <0%> (+0.03%) |
:arrow_up: |
#tests_posix_multi_threaded | 25.68% <23.07%> (+0.14%) |
:arrow_up: |
#tests_posix_single_threaded | 56.87% <80.76%> (+3.37%) |
:arrow_up: |
#tests_preload | 44.69% <50%> (+0.27%) |
:arrow_up: |
Impacted Files | Coverage Δ | |
---|---|---|
src/libpmemfile-posix/utils.h | 80% <ø> (ø) |
:arrow_up: |
src/libpmemfile-posix/utils.c | 68.51% <0%> (+0.18%) |
:arrow_up: |
src/libpmemfile-posix/write.c | 87.55% <100%> (+3.05%) |
:arrow_up: |
src/libpmemfile-posix/inode.c | 89.07% <100%> (ø) |
:arrow_up: |
src/libpmemfile-posix/rmdir.c | 94.94% <100%> (+2.87%) |
:arrow_up: |
src/libpmemfile-posix/statfs.c | 91.66% <100%> (+16.66%) |
:arrow_up: |
src/libpmemfile-posix/read.c | 94.44% <100%> (+1.97%) |
:arrow_up: |
src/libpmemfile-posix/chown.c | 98.97% <100%> (ø) |
:arrow_up: |
src/libpmemfile-posix/timestamps.c | 93.71% <100%> (+2.4%) |
:arrow_up: |
src/libpmemfile-posix/rename.c | 91.9% <100%> (+0.77%) |
:arrow_up: |
... and 16 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 7f181b5...7051b18. Read the comment docs.
Reviewed 16 of 16 files at r1. Review status: all files reviewed at latest revision, all discussions resolved.
Comments from Reviewable
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.
src/libpmemfile-posix/utils.c, line 50 at r1 (raw file):
pmemfile_timespec_t tm; if (clock_gettime(CLOCK_REALTIME, &tm)) { ERR("!clock_gettime");
Looks like overkill.
I'm thinking it is probably pointless to prepare for failures of clock_gettime. According to the docs, CLOCK_REALTIME
should be supported everywhere, so the only possible error would be EFAULT.
The buffer is on the stack, declared in the line right before the clock_gettime call.
The fault injection code for this returns EINVAL, which would suggest here that CLOCK_REALTIME is supported sometimes, and not supported perhaps a few microseconds later -- seems like pointless to prepare for this scenario, I mean, if that happens, then maybe adding 2 + 2 will result in 5, and we don't check for that either.
Comments from Reviewable
Review status: all files reviewed at latest revision, 1 unresolved discussion.
src/libpmemfile-posix/utils.c, line 50 at r1 (raw file):
Looks like overkill. I'm thinking it is probably pointless to prepare for failures of clock_gettime. According to the docs, `CLOCK_REALTIME` should be supported everywhere, so the only possible error would be EFAULT. The buffer is on the stack, declared in the line right before the clock_gettime call. The fault injection code for this returns EINVAL, which would suggest here that CLOCK_REALTIME is supported sometimes, and not supported perhaps a few microseconds later -- seems like pointless to prepare for this scenario, I mean, if that happens, then maybe adding 2 + 2 will result in 5, and we don't check for that either.
Ok. I handled this failure with an expectation that this function may need to be changed in the future. But I just realized none of those changes would mean that this function could fail...
Comments from Reviewable
Reviewed 5 of 16 files at r1, 17 of 20 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.
Comments from Reviewable
This change is