On 03/02/2016 03:43 PM, Lukas Slebodnik wrote:
On (02/03/16 15:13), Pavel Reichl wrote:
On 03/02/2016 03:07 PM, Lukas Slebodnik wrote:
On (02/03/16 15:02), Pavel Reichl wrote:
On 03/02/2016 02:59 PM, Lukas Slebodnik wrote:
On (02/03/16 13:45), Pavel Reichl wrote:
On 03/02/2016 01:10 PM, Lukas Slebodnik wrote:
On (02/03/16 13:02), Pavel Reichl wrote:
On 03/02/2016 12:53 PM, Lukas Slebodnik wrote:
On (02/03/16 12:48), Pavel Březina wrote:
On 03/01/2016 03:54 PM, Pavel Reichl wrote:
I added one more similar patch.
sss_idmap_calculate_range can accept domain SID or range identifier on
its input. Previous parameter name was misleading.
Ack to both. They can sure be squashed before pushing but I don't care.
I miss a link here.
Anyway I would like to see a Sumit opinin about renaming variables.
Because name of variables is one of hard things problems in IT
http://martinfowler.com/bliki/TwoHardThings.html
Might we could do an all hands meeting,
because we really don't want to underestimate such important change.
If the patch is not important then it does not make a sense to push it.
BTW You introduced one of bad argument names in the recent
commit 8babbeee01e67893af4828ddfc922ecac0be4197
Sure, I did that and now I see that it would be nice to use a different name.
Sumit is fine with changes.
Would you be so kind and could you send squased patch which
we can push.
If you insist on squashing the patches then please do it while pushing it.
I would but could you help me with commit mesage?
I'm sorry but for me it's the same problem as name of variables
http://martinfowler.com/bliki/TwoHardThings.html
"There are only two hard things in Computer Science: cache invalidation and
naming things."
Phil Karlto
I would appreciate if autor of the patch could do it.
LS
Lukas can you just push the patches as they are?
I would like but there is issue with separating changes to patches.
In general it make sense to have small patches. And each patch
should do one thing e.g. do not mix unrelated coding style with change of
logic. However if TWO patches do the same it does not make a sense to separate
them. In samba, they separate this if the change is in different part of code
e.g. ldb, tdm, talloc, samba3 ... The reason is bacporting of such changes.
But in our case the change is in one module.
Sumit and Pavel ACKed them independently and neither of them expressed
the desire of having them squashed.
Neither of them expresed that it's the best idea ever to have changes in two
patches.
Please update patch and I will send a link to CI because neither of reviewers
did it.
/sssd-devel@lists.fedorahosted.org
Squashed patch attached. Do you want Sumit and Pavel to review the patch again?
>From 54e04f26494fa37b0caa10613e0455b0650ee0c8 Mon Sep 17 00:00:00 2001
From: Pavel Reichl <prei...@redhat.com>
Date: Tue, 1 Mar 2016 08:41:24 -0500
Subject: [PATCH] IDMAP: Make parameter names more descriptive
Domain SID (not name) is part of identification string for helper range
in generate_sec_slice_name().
Use more generic name for range identifier when calculating range for
new slice in sss_idmap_calculate_range().
---
src/lib/idmap/sss_idmap.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/lib/idmap/sss_idmap.c b/src/lib/idmap/sss_idmap.c
index e3e9972b802748770a5f7440fa8ddc8ba75d3362..dbb152bb9afc3978f201c967ca31d6b67f7fc4b6 100644
--- a/src/lib/idmap/sss_idmap.c
+++ b/src/lib/idmap/sss_idmap.c
@@ -320,7 +320,7 @@ static bool check_dom_overlap(struct idmap_range_params *prim_range,
}
enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
- const char *dom_sid,
+ const char *range_id,
id_t *slice_num,
struct sss_idmap_range *_range)
{
@@ -362,8 +362,8 @@ enum idmap_error_code sss_idmap_calculate_range(struct sss_idmap_ctx *ctx,
*/
orig_slice = 0;
} else {
- /* Hash the domain sid string */
- hash_val = murmurhash3(dom_sid, strlen(dom_sid), 0xdeadbeef);
+ /* Hash the range identifier string */
+ hash_val = murmurhash3(range_id, strlen(range_id), 0xdeadbeef);
/* Now get take the modulus of the hash val and the max_slices
* to determine its optimal position in the range.
@@ -536,13 +536,13 @@ idmap_error_code dom_check_collision(struct idmap_domain_info *dom_list,
static char*
generate_sec_slice_name(struct sss_idmap_ctx *ctx,
- const char *domain_name, uint32_t rid)
+ const char *domain_sid, uint32_t rid)
{
const char *SEC_SLICE_NAME_FMT = "%s-%"PRIu32;
char *slice_name;
int len, len2;
- len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_name, rid);
+ len = snprintf(NULL, 0, SEC_SLICE_NAME_FMT, domain_sid, rid);
if (len <= 0) {
return NULL;
}
@@ -552,7 +552,7 @@ generate_sec_slice_name(struct sss_idmap_ctx *ctx,
return NULL;
}
- len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_name,
+ len2 = snprintf(slice_name, len + 1, SEC_SLICE_NAME_FMT, domain_sid,
rid);
if (len != len2) {
ctx->free_func(slice_name, ctx->alloc_pvt);
--
2.4.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
