Re: [SSSD] [PATCH] Allow sysdb_search_entry request to return more than one result
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/28/2009 02:55 PM, Sumit Bose wrote: Hi, this patch makes the sysdb_search_entry request more flexible by enableing it to return more than one result. I have modified the current callers so that they only take the first result and send a DEBUG message if there are more than one results. bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel Nack. In sysdb_search_entry_done() please move state-msgs_count++; above the talloc_realloc and use state-msgs_count + 1 for the size instead of +2. It reads better (one extra for the NULL). I'd also prefer if you used 'if (state-msgs_count == 0)' instead of 'if (!state-msgs)' in LDB_REPLY_DONE, because it's technically possible for state-msgs[0] == NULL and this test will still succeed (and be wrong). In the assorted _recv() functions, you implicitly return the first entry found. This is a distinct break from the previous functionality, where sysdb_search_entry_done() would have thrown an error if the count was 1. I think we need to continue reporting an error here instead of returning possibly incorrect data.* Besides, if we've gotten a multiple-value return for a base search, there's something seriously wrong and this needs to be reported. * The data could be incorrect because we don't necessarily know whether the LDB will always return the same user first. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkrplWYACgkQeiVVYja6o6MMcQCfVKSdyReoJ2tNFlSdLesFroqc npEAn0iMItMh9/J9O/RjsB0JSPzcjS6S =ekUQ -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Allow sysdb_search_entry request to return more than one result
On Thu, Oct 29, 2009 at 09:15:23AM -0400, Stephen Gallagher wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 10/28/2009 02:55 PM, Sumit Bose wrote:
Hi,
this patch makes the sysdb_search_entry request more flexible by
enableing it to return more than one result. I have modified the current
callers so that they only take the first result and send a DEBUG message
if there are more than one results.
bye,
Sumit
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
Nack.
In sysdb_search_entry_done() please move state-msgs_count++; above the
talloc_realloc and use state-msgs_count + 1 for the size instead of +2.
It reads better (one extra for the NULL). I'd also prefer if you used
'if (state-msgs_count == 0)' instead of 'if (!state-msgs)' in
LDB_REPLY_DONE, because it's technically possible for state-msgs[0] ==
NULL and this test will still succeed (and be wrong).
In the assorted _recv() functions, you implicitly return the first entry
found. This is a distinct break from the previous functionality, where
sysdb_search_entry_done() would have thrown an error if the count was
1. I think we need to continue reporting an error here instead of
returning possibly incorrect data.* Besides, if we've gotten a
multiple-value return for a base search, there's something seriously
wrong and this needs to be reported.
* The data could be incorrect because we don't necessarily know whether
the LDB will always return the same user first.
- --
Stephen Gallagher
RHCE 804006346421761
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkrplWYACgkQeiVVYja6o6MMcQCfVKSdyReoJ2tNFlSdLesFroqc
npEAn0iMItMh9/J9O/RjsB0JSPzcjS6S
=ekUQ
-END PGP SIGNATURE-
ok, the _recv functions return EFAULT now and the
'if (state-msgs_count == 0)' check is used. I haven't changed the
realloc section because it would result in a 'state-msgs_count -1'
in an index.
Thanks for the review.
bye,
Sumit
From 8c28f0936e02fdaeab6105c3e646aecce1027289 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 28 Oct 2009 19:42:06 +0100
Subject: [PATCH] Allow sysdb_search_entry request to return more than one result
---
server/db/sysdb.h |3 +-
server/db/sysdb_ops.c | 90 +--
server/tests/sysdb-tests.c | 112
3 files changed, 178 insertions(+), 27 deletions(-)
diff --git a/server/db/sysdb.h b/server/db/sysdb.h
index 5c15d3a..00a3378 100644
--- a/server/db/sysdb.h
+++ b/server/db/sysdb.h
@@ -321,7 +321,8 @@ struct tevent_req *sysdb_search_entry_send(TALLOC_CTX
*mem_ctx,
const char **attrs);
int sysdb_search_entry_recv(struct tevent_req *req,
TALLOC_CTX *mem_ctx,
-struct ldb_message **msg);
+size_t *msgs_size,
+struct ldb_message ***msgs);
/* Search User (by uid or name) */
struct tevent_req *sysdb_search_user_by_name_send(TALLOC_CTX *mem_ctx,
diff --git a/server/db/sysdb_ops.c b/server/db/sysdb_ops.c
index 1802234..acff5e5 100644
--- a/server/db/sysdb_ops.c
+++ b/server/db/sysdb_ops.c
@@ -203,6 +203,8 @@ struct sysdb_op_state {
bool ignore_not_found;
struct ldb_reply *ldbreply;
+size_t msgs_count;
+struct ldb_message **msgs;
};
static void sysdb_op_default_done(struct tevent_req *subreq)
@@ -322,6 +324,8 @@ struct tevent_req *sysdb_search_entry_send(TALLOC_CTX
*mem_ctx,
state-handle = handle;
state-ignore_not_found = false;
state-ldbreply = NULL;
+state-msgs_count = 0;
+state-msgs = NULL;
ret = ldb_build_search_req(ldbreq, handle-ctx-ldb, state,
base_dn, scope, filter, attrs,
@@ -354,6 +358,7 @@ static void sysdb_search_entry_done(struct tevent_req
*subreq)
struct sysdb_op_state *state = tevent_req_data(req,
struct sysdb_op_state);
struct ldb_reply *ldbreply;
+struct ldb_message **dummy;
int ret;
ret = sldb_request_recv(subreq, state, ldbreply);
@@ -365,27 +370,32 @@ static void sysdb_search_entry_done(struct tevent_req
*subreq)
switch (ldbreply-type) {
case LDB_REPLY_ENTRY:
-if (state-ldbreply) {
-DEBUG(1, (More than one reply for a base search ?!
- DB seems corrupted, aborting.));
-tevent_req_error(req, EFAULT);
+dummy = talloc_realloc(state, state-msgs,
+ struct ldb_message *,
+
Re: [SSSD] [PATCH] Allow sysdb_search_entry request to return more than one result
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/29/2009 10:02 AM, Sumit Bose wrote: On Thu, Oct 29, 2009 at 09:15:23AM -0400, Stephen Gallagher wrote: On 10/28/2009 02:55 PM, Sumit Bose wrote: Hi, this patch makes the sysdb_search_entry request more flexible by enableing it to return more than one result. I have modified the current callers so that they only take the first result and send a DEBUG message if there are more than one results. bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel Nack. In sysdb_search_entry_done() please move state-msgs_count++; above the talloc_realloc and use state-msgs_count + 1 for the size instead of +2. It reads better (one extra for the NULL). I'd also prefer if you used 'if (state-msgs_count == 0)' instead of 'if (!state-msgs)' in LDB_REPLY_DONE, because it's technically possible for state-msgs[0] == NULL and this test will still succeed (and be wrong). In the assorted _recv() functions, you implicitly return the first entry found. This is a distinct break from the previous functionality, where sysdb_search_entry_done() would have thrown an error if the count was 1. I think we need to continue reporting an error here instead of returning possibly incorrect data.* Besides, if we've gotten a multiple-value return for a base search, there's something seriously wrong and this needs to be reported. * The data could be incorrect because we don't necessarily know whether the LDB will always return the same user first. Ack ok, the _recv functions return EFAULT now and the 'if (state-msgs_count == 0)' check is used. I haven't changed the realloc section because it would result in a 'state-msgs_count -1' in an index. Thanks for the review. bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkrpq7YACgkQeiVVYja6o6NT6wCfTN+yoN5Xgq2LOAprpclXjsKB ATUAoJPs4PYBFiuMdf7ibCiOj0yAIhmq =wxmJ -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] Allow sysdb_search_entry request to return more than one result
Hi,
this patch makes the sysdb_search_entry request more flexible by
enableing it to return more than one result. I have modified the current
callers so that they only take the first result and send a DEBUG message
if there are more than one results.
bye,
Sumit
From 79149782d1dafc59f91fce3fcb305a2d652ecf7e Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Wed, 28 Oct 2009 19:42:06 +0100
Subject: [PATCH] Allow sysdb_search_entry request to return more than one result
---
server/db/sysdb.h |3 +-
server/db/sysdb_ops.c | 90 +--
server/tests/sysdb-tests.c | 112
3 files changed, 178 insertions(+), 27 deletions(-)
diff --git a/server/db/sysdb.h b/server/db/sysdb.h
index 5c15d3a..00a3378 100644
--- a/server/db/sysdb.h
+++ b/server/db/sysdb.h
@@ -321,7 +321,8 @@ struct tevent_req *sysdb_search_entry_send(TALLOC_CTX
*mem_ctx,
const char **attrs);
int sysdb_search_entry_recv(struct tevent_req *req,
TALLOC_CTX *mem_ctx,
-struct ldb_message **msg);
+size_t *msgs_size,
+struct ldb_message ***msgs);
/* Search User (by uid or name) */
struct tevent_req *sysdb_search_user_by_name_send(TALLOC_CTX *mem_ctx,
diff --git a/server/db/sysdb_ops.c b/server/db/sysdb_ops.c
index 1802234..3ebb03d 100644
--- a/server/db/sysdb_ops.c
+++ b/server/db/sysdb_ops.c
@@ -203,6 +203,8 @@ struct sysdb_op_state {
bool ignore_not_found;
struct ldb_reply *ldbreply;
+size_t msgs_count;
+struct ldb_message **msgs;
};
static void sysdb_op_default_done(struct tevent_req *subreq)
@@ -322,6 +324,8 @@ struct tevent_req *sysdb_search_entry_send(TALLOC_CTX
*mem_ctx,
state-handle = handle;
state-ignore_not_found = false;
state-ldbreply = NULL;
+state-msgs_count = 0;
+state-msgs = NULL;
ret = ldb_build_search_req(ldbreq, handle-ctx-ldb, state,
base_dn, scope, filter, attrs,
@@ -354,6 +358,7 @@ static void sysdb_search_entry_done(struct tevent_req
*subreq)
struct sysdb_op_state *state = tevent_req_data(req,
struct sysdb_op_state);
struct ldb_reply *ldbreply;
+struct ldb_message **dummy;
int ret;
ret = sldb_request_recv(subreq, state, ldbreply);
@@ -365,27 +370,32 @@ static void sysdb_search_entry_done(struct tevent_req
*subreq)
switch (ldbreply-type) {
case LDB_REPLY_ENTRY:
-if (state-ldbreply) {
-DEBUG(1, (More than one reply for a base search ?!
- DB seems corrupted, aborting.));
-tevent_req_error(req, EFAULT);
+dummy = talloc_realloc(state, state-msgs,
+ struct ldb_message *,
+ state-msgs_count + 2);
+if (dummy == NULL) {
+tevent_req_error(req, ENOMEM);
return;
}
+state-msgs = dummy;
-/* save the entry so that it can be retrieved by the caller */
-state-ldbreply = ldbreply;
+state-msgs[state-msgs_count + 1] = NULL;
-/* just return, wait for a LDB_REPLY_DONE entry */
+state-msgs[state-msgs_count] = talloc_steal(state-msgs,
+ ldbreply-message);
+state-msgs_count++;
+
+talloc_zfree(ldbreply);
return;
case LDB_REPLY_DONE:
-if (!state-ldbreply) {
-talloc_zfree(ldbreply);
+talloc_zfree(subreq);
+talloc_zfree(ldbreply);
+if (!state-msgs) {
DEBUG(6, (Error: Entry not Found!\n));
tevent_req_error(req, ENOENT);
return;
}
-talloc_zfree(ldbreply);
return tevent_req_done(req);
default:
@@ -399,7 +409,8 @@ static void sysdb_search_entry_done(struct tevent_req
*subreq)
int sysdb_search_entry_recv(struct tevent_req *req,
TALLOC_CTX *mem_ctx,
-struct ldb_message **msg)
+size_t *msgs_count,
+struct ldb_message ***msgs)
{
struct sysdb_op_state *state = tevent_req_data(req,
struct sysdb_op_state);
@@ -410,7 +421,8 @@ int sysdb_search_entry_recv(struct tevent_req *req,
return err;
}
-*msg = talloc_move(mem_ctx, state-ldbreply-message);
+*msgs_count = state-msgs_count;
+*msgs = talloc_move(mem_ctx, state-msgs);
return EOK;
}
@@ -427,7 +439,8 @@ struct sysdb_search_user_state {
const char *filter;
int scope;
-struct ldb_message *msg;
+size_t msgs_count;
+struct ldb_message **msgs;
};
static void sysdb_search_user_cont(struct tevent_req *subreq);
@@ -453,7 +466,8 @@
