On Thu, Dec 20, 2012 at 02:05:38PM +0100, Ondrej Kos wrote:
Here are two patches for the SYSDB:
[PATCH 1/2] SYSDB: replace ghost users properly
this fixes
https://fedorahosted.org/sssd/ticket/1714
[PATCH 2/2] SYSDB: split sysdb_add_user
splits add_user code to more readable blocks
Patches are attached
I haven't tested the patches yet, just by visual inspection:
From 54ed4f83cd28eb8a08a7f402847b22057f0a10f6 Mon Sep 17 00:00:00
2001
From: Ondrej Kos <okos(a)redhat.com>
Date: Thu, 20 Dec 2012 13:15:31 +0100
Subject: [PATCH 1/2] SYSDB: replace ghost users properly
https://fedorahosted.org/sssd/ticket/1714
We were trying to delete all existing ghosts for users name and alias
even if they weren't in the database. Now only the matching entries are
removed and replaced by member.
---
src/db/sysdb_ops.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 4bbc76c5c39501a4d878a1d41faa81919a23228f..fbd5c413581a72240b798ba68123efef8d470f09
100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -74,6 +74,22 @@ static uint32_t get_attr_as_uint32(struct ldb_message *msg, const char
*attr)
return l;
}
+static int find_name_in_ghosts(const char *name,
+ struct ldb_message_element *ghosts)
This function sounds like it should have been using bool?
+{
+ int ret = 0;
+ int i;
+
+ for (i = 0; i < ghosts->num_values; i++) {
+ if (strcmp(name, (char *) ghosts->values[i].data)) {
+ ret = i;
+ break;
+ }
+ }
+
+ return ret;
+}
+
#define ERROR_OUT(v, r, l) do { v = r; goto l; } while(0)
@@ -879,6 +895,7 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
uint32_t id;
int ret, i, j;
bool add_member = false;
+ struct ldb_message_element *ghosts;
struct sss_domain_info *domain = sysdb->domain;
@@ -1081,18 +1098,20 @@ int sysdb_add_user(struct sysdb_ctx *sysdb,
if (ret) goto done;
}
- ret = add_string(msg, LDB_FLAG_MOD_DELETE, SYSDB_GHOST, name);
- if (ret) goto done;
+ ghosts = NULL;
+ ghosts = ldb_msg_find_element(groups[i], SYSDB_GHOST);
- /* Delete aliases from the ghost attribute as well */
- for (j = 0; j < alias_el->num_values; j++) {
- if (strcmp((const char *)alias_el->values[j].data, name) == 0) {
- continue;
- }
- ret = ldb_msg_add_string(msg, SYSDB_GHOST,
- (char *) alias_el->values[j].data);
- if (ret != LDB_SUCCESS) {
- ERROR_OUT(ret, EINVAL, done);
+ if (ghosts != NULL) {
+ if (find_name_in_ghosts(name, ghosts) != 0)
+ {
Code style, don't open with { on a newline
+ ret = add_string(msg, LDB_FLAG_MOD_DELETE,
SYSDB_GHOST, name);
+ } else {
+ for (j = 0; j < alias_el->num_values; j++) {
+ if (find_name_in_ghosts((const char *)alias_el->values[j].data,
ghosts) != 0) {
+ ret = add_string(msg, LDB_FLAG_MOD_DELETE, SYSDB_GHOST, (char
*)alias_el->values[j].data);
+ break;
I wonder if there could be a group that would reference a single user
with both name and alias. Then we would probably only remove one alias
in this code. The group would be broken, but we should be defensive.
+ }
+ }
}
}
--
1.7.11.7