add pool_invalid_argument exception

[JanDorniak99]

This modification will allow us to better handle exceptions in pmemkv. pmem/pmemkv#942

---

This change is 

[lukaszstolarczuk]

Reviewed 4 of 6 files at r1.
Reviewable status: 4 of 6 files reviewed, 3 unresolved discussions (waiting on @JanDorniak99)

---

include/libpmemobj++/pool.hpp, line 117 at r1 (raw file):

				throw pmem::pool_invalid_argument(
					"Failed opening pool")
					.with_pmemobj_errormsg();

Can you please paste the example of full message of this exception, when thrown...?

---

include/libpmemobj++/pool.hpp, line 157 at r1 (raw file):

#endif
		if (pop == nullptr) {
			if (errno == EINVAL || errno == EFBIG ||

can we avoid copy-pasting these if's..?

---

tests/pool/pool.cpp, line 26 at r1 (raw file):

};

/* emulate no more space in the memory */

do we have test for too small pool size? Actually, pls verify if we have enough tests for pool create/open right now; perhaps we're already missing some..

---

tests/pool/pool.cpp, line 86 at r1 (raw file):

		UT_OUT("%s: pool::open: %s", path, e.what());
		return;
	}

shouldn't we have here tests for both exceptions?

[KFilipek]

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @JanDorniak99)

---

include/libpmemobj++/pool.hpp, line 115 at r1 (raw file):

Quoted 11 lines of code…

		if (pop == nullptr) {
			if (errno == EINVAL || errno == EFBIG ||
			    errno == ENOENT) {
				throw pmem::pool_invalid_argument(
					"Failed opening pool")
					.with_pmemobj_errormsg();
			} else {
				throw pmem::pool_error("Failed opening pool")
					.with_pmemobj_errormsg();
			}
		}

Every errno code has own meaning, in this case we get new exception with the same effect, but only change is an exclusion the case "internal pool error". Can we add reason why it failed to string? "Failed opening pool: no such file or directory" in ENOENT etc.

---

include/libpmemobj++/pool.hpp, line 215 at r1 (raw file):

Quoted 11 lines of code…

		if (pop == nullptr) {
			if (errno == EINVAL || errno == EFBIG ||
			    errno == ENOENT) {
				throw pmem::pool_invalid_argument(
					"Failed opening pool")
					.with_pmemobj_errormsg();
			} else {
				throw pmem::pool_error("Failed opening pool")
					.with_pmemobj_errormsg();
			}
		}

Duplicated 4 times code should be moved to function

[codecov]

Codecov Report

Merging #1047 (d0f80c0) into master (f22c61e) will decrease coverage by 0.09%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##           master    #1047      +/-   ##
==========================================
- Coverage   94.12%   94.02%   -0.10%     
==========================================
  Files          48       48              
  Lines        4712     4685      -27     
==========================================
- Hits         4435     4405      -30     
- Misses        277      280       +3     

Flag	Coverage Δ
tests_clang_debug_cpp17	93.46% <76.92%> (-0.07%)	⬇️
tests_gcc_debug	91.85% <83.33%> (-0.10%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
include/libpmemobj++/pexceptions.hpp	44.61% <57.14%> (+1.75%)	⬆️
include/libpmemobj++/pool.hpp	85.89% <100.00%> (-1.99%)	⬇️
include/libpmemobj++/mutex.hpp	79.31% <0.00%> (-6.90%)	⬇️
...ude/libpmemobj++/container/concurrent_hash_map.hpp	93.26% <0.00%> (-0.95%)	⬇️
include/libpmemobj++/transaction.hpp	81.65% <0.00%> (-0.92%)	⬇️
include/libpmemobj++/detail/life.hpp	100.00% <0.00%> (+2.00%)	⬆️

---

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 f22c61e...d0f80c0. Read the comment docs.

[igchor]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @JanDorniak99 and @KFilipek)

---

include/libpmemobj++/pool.hpp, line 115 at r1 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

		if (pop == nullptr) {
			if (errno == EINVAL || errno == EFBIG ||
			    errno == ENOENT) {
				throw pmem::pool_invalid_argument(
					"Failed opening pool")
					.with_pmemobj_errormsg();
			} else {
				throw pmem::pool_error("Failed opening pool")
					.with_pmemobj_errormsg();
			}
		}

Every errno code has own meaning, in this case we get new exception with the same effect, but only change is an exclusion the case "internal pool error". Can we add reason why it failed to string? "Failed opening pool: no such file or directory" in ENOENT etc.

It will be added automatically by the pmemobj_errormsg()

---

include/libpmemobj++/pool.hpp, line 215 at r1 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

		if (pop == nullptr) {
			if (errno == EINVAL || errno == EFBIG ||
			    errno == ENOENT) {
				throw pmem::pool_invalid_argument(
					"Failed opening pool")
					.with_pmemobj_errormsg();
			} else {
				throw pmem::pool_error("Failed opening pool")
					.with_pmemobj_errormsg();
			}
		}

Duplicated 4 times code should be moved to function

+1

[JanDorniak99]

Reviewable status: 3 of 12 files reviewed, 5 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

---

include/libpmemobj++/pool.hpp, line 115 at r1 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

		if (pop == nullptr) {
			if (errno == EINVAL || errno == EFBIG ||
			    errno == ENOENT) {
				throw pmem::pool_invalid_argument(
					"Failed opening pool")
					.with_pmemobj_errormsg();
			} else {
				throw pmem::pool_error("Failed opening pool")
					.with_pmemobj_errormsg();
			}
		}

Every errno code has own meaning, in this case we get new exception with the same effect, but only change is an exclusion the case "internal pool error". Can we add reason why it failed to string? "Failed opening pool: no such file or directory" in ENOENT etc.

I think that is already included in pmemobj_errormsg(). Example msg after not catching pool_invalid_argument exception: Failed creating pool: open "/home/nvms/Work/libpmemobj-cpp/build/test/pool_4_none/non-existent-dir/testfile": No such file or directory

---

include/libpmemobj++/pool.hpp, line 117 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

Can you please paste the example of full message of this exception, when thrown...?

Failed creating pool: open "/home/nvms/Work/libpmemobj-cpp/build/test/pool_4_none/non-existent-dir/testfile": No such file or directory

---

include/libpmemobj++/pool.hpp, line 157 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

can we avoid copy-pasting these if's..?

Done.

---

include/libpmemobj++/pool.hpp, line 215 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

+1

Done.

---

tests/pool/pool.cpp, line 26 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

do we have test for too small pool size? Actually, pls verify if we have enough tests for pool create/open right now; perhaps we're already missing some..

Done.

---

tests/pool/pool.cpp, line 86 at r1 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

shouldn't we have here tests for both exceptions?

Done.

[JanDorniak99]

Reviewable status: 3 of 12 files reviewed, 5 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

---

include/libpmemobj++/pool.hpp, line 115 at r1 (raw file):

Previously, JanDorniak99 (Jan Dorniak) wrote…

I think that is already included in pmemobj_errormsg(). Example msg after not catching pool_invalid_argument exception: Failed creating pool: open "/home/nvms/Work/libpmemobj-cpp/build/test/pool_4_none/non-existent-dir/testfile": No such file or directory

Done.

[igchor]

Reviewed 1 of 6 files at r1.
Reviewable status: 3 of 12 files reviewed, 5 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

---

tests/pool/pool_9.cmake, line 10 at r2 (raw file):

# test for opening pool without access
execute(${TEST_EXECUTABLE} c ${DIR}/testfile "test" 20 0600)
execute_process(COMMAND chmod 000 ${DIR}/testfile)

does this work on windows? if not, I guess it should be disabled

[KFilipek]

Reviewed 9 of 9 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @JanDorniak99 and @lukaszstolarczuk)

---

include/libpmemobj++/pool.hpp, line 191 at r2 (raw file):

	{
		pmemobjpool *pop = pmemobj_openW(path.c_str(), layout.c_str());

empty line, that nothing improves in general

---

include/libpmemobj++/pool.hpp, line 223 at r2 (raw file):

						   size, mode);

		check_pool(pop, "creating");

remove empty line

[JanDorniak99]

Reviewable status: 6 of 10 files reviewed, 5 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

---

include/libpmemobj++/pool.hpp, line 191 at r2 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

empty line, that nothing improves in general

Done.

---

include/libpmemobj++/pool.hpp, line 223 at r2 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

remove empty line

Done.

[lukaszstolarczuk]

Reviewed 3 of 9 files at r2, 5 of 6 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @JanDorniak99 and @KFilipek)

---

tests/CMakeLists.txt, line 164 at r3 (raw file):

add_test_generic(NAME pool CASE 4 TRACERS none)
add_test_generic(NAME pool CASE 5 TRACERS none)
add_test_generic(NAME pool CASE 7 TRACERS none)

why 7 not 6? ;)

[JanDorniak99]

Reviewable status: 9 of 10 files reviewed, 3 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

---

tests/CMakeLists.txt, line 164 at r3 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

why 7 not 6? ;)

Done.

[karczex]

Reviewable status: 9 of 10 files reviewed, 3 unresolved discussions (waiting on @KFilipek and @lukaszstolarczuk)

[lukaszstolarczuk]

Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @KFilipek)

[JanDorniak99]

Pls don't merge this now, I probably found one missing check.

[lukaszstolarczuk]

Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @KFilipek)

[KFilipek]

Reviewed 2 of 6 files at r3, 1 of 1 files at r4, 1 of 1 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @JanDorniak99)