On (05/02/16 16:56), Pavel Reichl wrote:
Hopefully the last one.
From 58b06b8fee18c4242e301ecf71f92a77f6ba6e7b Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Fri, 22 Jan 2016 08:34:14 -0500
Subject: [PATCH] IDMAP: Add test to validate off by one bug
Resolves:
https://fedorahosted.org/sssd/ticket/2922
---
src/tests/cmocka/test_sss_idmap.c | 125 ++++++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
diff --git a/src/tests/cmocka/test_sss_idmap.c b/src/tests/cmocka/test_sss_idmap.c
index 00e03ffd9ab1532fb55795b9935b254c8a89ec16..2c99a09e6d11e8d731860eaad94eda0900fafabe
100644
--- a/src/tests/cmocka/test_sss_idmap.c
+++ b/src/tests/cmocka/test_sss_idmap.c
@@ -43,6 +43,17 @@
#define TEST_OFFSET 1000000
#define TEST_OFFSET_STR "1000000"
+#define TEST_2922_MIN_ID 1842600000
+#define TEST_2922_MAX_ID 1842799999
+#define TEST_2922_MAX_ID_PLUS_ONE (TEST_2922_MAX_ID + 1)
+
+#define TEST_2922_DFL_SLIDE 9212
+#define TEST_2922_FIRST_SID TEST_DOM_SID"-0"
+/* Last SID = first SID + (default) rangesize -1 */
+#define TEST_2922_LAST_SID TEST_DOM_SID"-199999"
+/* Last SID = first SID + rangesize */
+#define TEST_2922_LAST_SID_PLUS_ONE TEST_DOM_SID"-200000"
+
We should prefer to use constants instead of #defines.
Constants are more type safe.
struct test_ctx {
TALLOC_CTX *mem_idmap;
struct sss_idmap_ctx *idmap_ctx;
@@ -128,6 +139,53 @@ static int setup_ranges(struct test_ctx *test_ctx, bool
external_mapping,
return 0;
}
+static int setup_ranges_2922(struct test_ctx *test_ctx)
+{
+ struct sss_idmap_range range;
+ enum idmap_error_code err;
+ const char *name;
+ const char *sid;
+ /* Pick a new slice. */
+ id_t slice_num = -1;
+
+ if (test_ctx == NULL) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "test context is NULL\n");
+ return 1;
+ }
+
+ name = TEST_DOM_NAME;
+ sid = TEST_DOM_SID;
+
+ err = sss_idmap_calculate_range(test_ctx->idmap_ctx, sid, &slice_num,
+ &range);
+ if (err != IDMAP_SUCCESS) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "sss_idmap_calculate_range failed: exp: [%d] but got [%d]\n",
+ IDMAP_SUCCESS, err);
+ return 1;
Please use different numbers for different failures.
Persoanly, I do not like theusage of such error handling in tests.
But that was a desigh decision of cmocka developers
which I do not consider as the right one.
Old versions of cmocka allowed to use assertions in setup and teradown
function. The same applies to check framework. Such decision add
unnecassary complexity to the review process. Reviewer need to check
that different error codes are returned or debug message is logged
with proper debug level. But that's the separete discusssion.
+ }
+
+ /* Range computation should be deterministic. Lets validate that. */
+ if (range.min != TEST_2922_MIN_ID
+ || range.max != TEST_2922_MAX_ID
+ || slice_num != TEST_2922_DFL_SLIDE) {
+ DEBUG(SSSDBG_FATAL_FAILURE, "Failed to compute range correctly.\n");
+ return 1;
+ }
+
+ err = sss_idmap_add_domain_ex(test_ctx->idmap_ctx, name, sid, &range,
+ NULL, 0, false /* No external mapping */);
+ if (err != IDMAP_SUCCESS) {
+ DEBUG(SSSDBG_FATAL_FAILURE,
+ "sss_idmap_add_domain_ex failed: exp: [%d] but got [%d]\n",
+ IDMAP_SUCCESS, err);
+ return 1;
+ }
+
+ return 0;
+}
+
LS