Re: [Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context

2015-03-18 Thread Jakub Hrozek 
On Wed, Mar 18, 2015 at 10:58:51AM +0100, Sumit Bose wrote:
 Please find attached a new version where the typo is fixed.
 
 bye,
 Sumit

ACK

I think the IPA gatekeepers shoudl feel free to just fix these trivial
errors before pushing in the future.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context

2015-03-18 Thread Sumit Bose 
On Fri, Mar 13, 2015 at 03:17:10PM +0100, Jakub Hrozek wrote:
 On Fri, Mar 13, 2015 at 11:55:09AM +0100, Sumit Bose wrote:
  On Wed, Mar 04, 2015 at 06:35:22PM +0100, Sumit Bose wrote:
   Hi,
   
   this patch series improves error reporting of the extdom plugin
   especially on the client side. Currently there is only SSSD ticket
   https://fedorahosted.org/sssd/ticket/2463 . Shall I create a
   corresponding FreeIPA ticket as well?
   
   In the third patch I already added a handful of new error messages.
   Suggestions for more messages are welcome.
   
   bye,
   Sumit
  
  Rebased versions attached.
  
  bye,
  Sumit
 
 The patches look good and work fine. I admit I cheated a bit and
 modified the code to return a failure. Then I saw on the client:
 
 [sssd[be[ipa.example.com]]] [ipa_s2n_exop_send] (0x0400): Executing extended 
 operation
 [sssd[be[ipa.example.com]]] [ipa_s2n_exop_done] (0x0040): 
 ldap_extended_operation result: Operations
 error(1), Failed to create fully qualified name.
 [sssd[be[ipa.example.com]]] [ipa_s2n_exop_done] (0x0040): 
 ldap_extended_operation failed, server logs might contain more details.
 [sssd[be[ipa.example.com]]] [ipa_s2n_get_user_done] (0x0040): s2n exop 
 request failed.
 
 I just saw one typo:
 
  @@ -918,6 +934,7 @@ static int handle_sid_request(struct ipa_extdom_ctx 
  *ctx,
   ret = sss_nss_getorigbyname(pwd.pw_name, kv_list, id_type);
   if (ret != 0 || !(id_type == SSS_ID_TYPE_UID
   || id_type == SSS_ID_TYPE_BOTH)) {
  +set_err_msg(req, Failed ot read original data);
  ~
   if (ret == ENOENT) {
   ret = LDAP_NO_SUCH_OBJECT;
   } else {
 
 And a compilation warning caused by previous patches.
 
 So ACK provided the typo is fixed prior to pushing the patch.
 

Please find attached a new version where the typo is fixed.

bye,
Sumit
From 9638b0a61144a481a0a2c8757d4e3a10e1f44b12 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:52:10 +0100
Subject: [PATCH 137/139] extdom: add err_msg member to request context

---
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h| 1 +
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 1 +
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c  | 5 -
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
d4c851169ddadc869a59c53075f9fc7f33321085..421f6c6ea625aba2db7e9ffc84115b3647673699
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -116,6 +116,7 @@ struct extdom_req {
 gid_t gid;
 } posix_gid;
 } data;
+char *err_msg;
 };
 
 struct extdom_res {
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
47bcb179f04e08c64d92f55809b84f2d59622344..c2fd42f13fca97587ddc4c12b560e590462f121b
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -356,6 +356,7 @@ void free_req_data(struct extdom_req *req)
 break;
 }
 
+free(req-err_msg);
 free(req);
 }
 
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index 
dc7785877fc321ddaa5b6967d1c1b06cb454bbbf..708d0e4a2fc9da4f87a24a49c945587049f7280f
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -149,12 +149,15 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
 rc = LDAP_SUCCESS;
 
 done:
-free_req_data(req);
+if (req-err_msg != NULL) {
+err_msg = req-err_msg;
+}
 if (err_msg != NULL) {
 LOG(%s, err_msg);
 }
 slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
 ber_bvfree(ret_val);
+free_req_data(req);
 return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
 }
 
-- 
2.1.0

From b52bc91cf9e10a779446bf399f138eb3929cfb55 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:53:06 +0100
Subject: [PATCH 138/139] extdom: add add_err_msg() with test

---
 .../ipa-extdom-extop/ipa_extdom.h  |  1 +
 .../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 43 ++
 .../ipa-extdom-extop/ipa_extdom_common.c   | 23 
 3 files changed, 67 insertions(+)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
421f6c6ea625aba2db7e9ffc84115b3647673699..0d5d55d2fb0ece95466b0225b145a4edeef18efa
 100644
---

Re: [Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context

2015-03-18 Thread Tomas Babej 


On 03/18/2015 11:23 AM, Jakub Hrozek wrote:

On Wed, Mar 18, 2015 at 10:58:51AM +0100, Sumit Bose wrote:

Please find attached a new version where the typo is fixed.

bye,
Sumit

ACK

I think the IPA gatekeepers shoudl feel free to just fix these trivial
errors before pushing in the future.



Pushed to master: 6cc6a3ceec98bf2ee4f9dd776d70bd3390acb384

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context

2015-03-13 Thread Sumit Bose 
On Wed, Mar 04, 2015 at 06:35:22PM +0100, Sumit Bose wrote:
 Hi,
 
 this patch series improves error reporting of the extdom plugin
 especially on the client side. Currently there is only SSSD ticket
 https://fedorahosted.org/sssd/ticket/2463 . Shall I create a
 corresponding FreeIPA ticket as well?
 
 In the third patch I already added a handful of new error messages.
 Suggestions for more messages are welcome.
 
 bye,
 Sumit

Rebased versions attached.

bye,
Sumit
From e402a733322c68db0f808d3386a27e5fd9bc177b Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:52:10 +0100
Subject: [PATCH 137/139] extdom: add err_msg member to request context

---
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h| 1 +
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 1 +
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c  | 5 -
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
d4c851169ddadc869a59c53075f9fc7f33321085..421f6c6ea625aba2db7e9ffc84115b3647673699
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -116,6 +116,7 @@ struct extdom_req {
 gid_t gid;
 } posix_gid;
 } data;
+char *err_msg;
 };
 
 struct extdom_res {
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
47bcb179f04e08c64d92f55809b84f2d59622344..c2fd42f13fca97587ddc4c12b560e590462f121b
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -356,6 +356,7 @@ void free_req_data(struct extdom_req *req)
 break;
 }
 
+free(req-err_msg);
 free(req);
 }
 
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index 
dc7785877fc321ddaa5b6967d1c1b06cb454bbbf..708d0e4a2fc9da4f87a24a49c945587049f7280f
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -149,12 +149,15 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
 rc = LDAP_SUCCESS;
 
 done:
-free_req_data(req);
+if (req-err_msg != NULL) {
+err_msg = req-err_msg;
+}
 if (err_msg != NULL) {
 LOG(%s, err_msg);
 }
 slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
 ber_bvfree(ret_val);
+free_req_data(req);
 return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
 }
 
-- 
2.1.0

From 6a6a18313745e5e50b629009a74f7b0ad1975fe2 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:53:06 +0100
Subject: [PATCH 138/139] extdom: add add_err_msg() with test

---
 .../ipa-extdom-extop/ipa_extdom.h  |  1 +
 .../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 43 ++
 .../ipa-extdom-extop/ipa_extdom_common.c   | 23 
 3 files changed, 67 insertions(+)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
421f6c6ea625aba2db7e9ffc84115b3647673699..0d5d55d2fb0ece95466b0225b145a4edeef18efa
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -185,4 +185,5 @@ int getgrnam_r_wrapper(size_t buf_max, const char *name,
struct group *grp, char **_buf, size_t *_buf_len);
 int getgrgid_r_wrapper(size_t buf_max, gid_t gid,
struct group *grp, char **_buf, size_t *_buf_len);
+void set_err_msg(struct extdom_req *req, const char *format, ...);
 #endif /* _IPA_EXTDOM_H_ */
diff --git 
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
index 
d5bacd7e8c9dc0a71eea70162406c7e5b67384ad..586b58b0fd4c7610e9cb4643b6dae04f9d22b8ab
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
@@ -213,6 +213,47 @@ void test_getgrgid_r_wrapper(void **state)
 free(buf);
 }
 
+void extdom_req_setup(void **state)
+{
+struct extdom_req *req;
+
+req = calloc(sizeof(struct extdom_req), 1);
+assert_non_null(req);
+
+*state = req;
+}
+
+void extdom_req_teardown(void **state)
+{
+struct extdom_req *req;
+
+req = (struct extdom_req *) *state;
+
+free_req_data(req);
+}
+
+void test_set_err_msg(void **state)
+{
+struct extdom_req *req;
+
+req = (struct extdom_req *) *state;
+assert_null(req-err_msg);
+
+set_err_msg(NULL, NULL);
+assert_null(req-err_msg);
+
+set_err_msg(req, NULL);
+

Re: [Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context

2015-03-13 Thread Jakub Hrozek 
On Fri, Mar 13, 2015 at 11:55:09AM +0100, Sumit Bose wrote:
 On Wed, Mar 04, 2015 at 06:35:22PM +0100, Sumit Bose wrote:
  Hi,
  
  this patch series improves error reporting of the extdom plugin
  especially on the client side. Currently there is only SSSD ticket
  https://fedorahosted.org/sssd/ticket/2463 . Shall I create a
  corresponding FreeIPA ticket as well?
  
  In the third patch I already added a handful of new error messages.
  Suggestions for more messages are welcome.
  
  bye,
  Sumit
 
 Rebased versions attached.
 
 bye,
 Sumit

The patches look good and work fine. I admit I cheated a bit and
modified the code to return a failure. Then I saw on the client:

[sssd[be[ipa.example.com]]] [ipa_s2n_exop_send] (0x0400): Executing extended 
operation
[sssd[be[ipa.example.com]]] [ipa_s2n_exop_done] (0x0040): 
ldap_extended_operation result: Operations
error(1), Failed to create fully qualified name.
[sssd[be[ipa.example.com]]] [ipa_s2n_exop_done] (0x0040): 
ldap_extended_operation failed, server logs might contain more details.
[sssd[be[ipa.example.com]]] [ipa_s2n_get_user_done] (0x0040): s2n exop request 
failed.

I just saw one typo:

 @@ -918,6 +934,7 @@ static int handle_sid_request(struct ipa_extdom_ctx *ctx,
  ret = sss_nss_getorigbyname(pwd.pw_name, kv_list, id_type);
  if (ret != 0 || !(id_type == SSS_ID_TYPE_UID
  || id_type == SSS_ID_TYPE_BOTH)) {
 +set_err_msg(req, Failed ot read original data);
 ~
  if (ret == ENOENT) {
  ret = LDAP_NO_SUCH_OBJECT;
  } else {

And a compilation warning caused by previous patches.

So ACK provided the typo is fixed prior to pushing the patch.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCHES 137-139] extdom: add err_msg member to request context

2015-03-04 Thread Sumit Bose 
Hi,

this patch series improves error reporting of the extdom plugin
especially on the client side. Currently there is only SSSD ticket
https://fedorahosted.org/sssd/ticket/2463 . Shall I create a
corresponding FreeIPA ticket as well?

In the third patch I already added a handful of new error messages.
Suggestions for more messages are welcome.

bye,
Sumit
From 2e8e4abb7e79d44f0ce0560daeb7696d9641a684 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:52:10 +0100
Subject: [PATCH 137/139] extdom: add err_msg member to request context

---
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h| 1 +
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c | 1 +
 daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c  | 5 -
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
d4c851169ddadc869a59c53075f9fc7f33321085..421f6c6ea625aba2db7e9ffc84115b3647673699
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -116,6 +116,7 @@ struct extdom_req {
 gid_t gid;
 } posix_gid;
 } data;
+char *err_msg;
 };
 
 struct extdom_res {
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
448710993f551298d3a4cdcc19371b8432773478..27c1313cb1f6f614b0c74992d4a768c3051a86ae
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -360,6 +360,7 @@ void free_req_data(struct extdom_req *req)
 break;
 }
 
+free(req-err_msg);
 free(req);
 }
 
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
index 
e53f968db040a37fbd6a193f87b3671eeabda89d..a70ed20f1816a7e00385edae8a81dd5dad9e9362
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_extop.c
@@ -145,11 +145,14 @@ static int ipa_extdom_extop(Slapi_PBlock *pb)
 rc = LDAP_SUCCESS;
 
 done:
-free_req_data(req);
+if (req-err_msg != NULL) {
+err_msg = req-err_msg;
+}
 if (err_msg != NULL) {
 LOG(%s, err_msg);
 }
 slapi_send_ldap_result(pb, rc, NULL, err_msg, 0, NULL);
+free_req_data(req);
 return SLAPI_PLUGIN_EXTENDED_SENT_RESULT;
 }
 
-- 
2.1.0

From 80406c884eddeb2ebf35bd12a7ed1a8ddd4af2fe Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Mon, 2 Feb 2015 00:53:06 +0100
Subject: [PATCH 138/139] extdom: add add_err_msg() with test

---
 .../ipa-extdom-extop/ipa_extdom.h  |  1 +
 .../ipa-extdom-extop/ipa_extdom_cmocka_tests.c | 43 ++
 .../ipa-extdom-extop/ipa_extdom_common.c   | 23 
 3 files changed, 67 insertions(+)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
421f6c6ea625aba2db7e9ffc84115b3647673699..0d5d55d2fb0ece95466b0225b145a4edeef18efa
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -185,4 +185,5 @@ int getgrnam_r_wrapper(size_t buf_max, const char *name,
struct group *grp, char **_buf, size_t *_buf_len);
 int getgrgid_r_wrapper(size_t buf_max, gid_t gid,
struct group *grp, char **_buf, size_t *_buf_len);
+void set_err_msg(struct extdom_req *req, const char *format, ...);
 #endif /* _IPA_EXTDOM_H_ */
diff --git 
a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
index 
be736dd9c5af4d0b632f1dbc55033fdf738bad46..0ca67030bf667ecd559443842cda166354ce54ce
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_cmocka_tests.c
@@ -213,6 +213,47 @@ void test_getgrgid_r_wrapper(void **state)
 free(buf);
 }
 
+void extdom_req_setup(void **state)
+{
+struct extdom_req *req;
+
+req = calloc(sizeof(struct extdom_req), 1);
+assert_non_null(req);
+
+*state = req;
+}
+
+void extdom_req_teardown(void **state)
+{
+struct extdom_req *req;
+
+req = (struct extdom_req *) *state;
+
+free_req_data(req);
+}
+
+void test_set_err_msg(void **state)
+{
+struct extdom_req *req;
+
+req = (struct extdom_req *) *state;
+assert_null(req-err_msg);
+
+set_err_msg(NULL, NULL);
+assert_null(req-err_msg);
+
+set_err_msg(req, NULL);
+assert_null(req-err_msg);
+
+set_err_msg(req, Test [%s][%d]., ABCD, 1234);
+assert_non_null(req-err_msg);
+