On Wed, May 13, 2015 at 04:51:53PM +0200, Pavel Březina wrote:
On 05/11/2015 05:58 PM, Jakub Hrozek wrote:
>Thanks for the patches. They work quite well!
>
>One bug I found is that if you query a nonexistant object path with
>GetAll, then all subsequent queries block. Maybe we don't finish some
>request on error? This is what I tried:
>[jhrozek@client] sssd $ [(review)] dbus-send --print-reply --system
--dest=org.freedesktop.sssd.infopipe
/org/freedesktop/sssd/infopipe/Users/redhat_2ecom/10327
org.freedesktop.DBus.Properties.GetAll string:org.freedesktop.sssd.infopipe.Users.User
>Error org.freedesktop.DBus.Error.NoReply: Message did not receive a reply (timeout by
message bus)
>[jhrozek@client] sssd $ [(review)] dbus-send --print-reply --system
--dest=org.freedesktop.sssd.infopipe
/org/freedesktop/sssd/infopipe/Users/linux_2etest/465400000
org.freedesktop.DBus.Properties.GetAll string:org.freedesktop.sssd.infopipe.Users.User
>Error org.freedesktop.DBus.Error.TimedOut: Activation of
org.freedesktop.sssd.infopipe timed out
>
>The first objectpath is a copy-n-paste error from a different machine,
>the second is legitimate. All works well unless I use the nonexistant path.
Container wasn't closed properly and it broke the bus.
diff --git a/src/sbus/sssd_dbus_invokers.c b/src/sbus/sssd_dbus_invokers.c
index b2f0de0..f4d4ba3 100644
--- a/src/sbus/sssd_dbus_invokers.c
+++ b/src/sbus/sssd_dbus_invokers.c
@@ -336,6 +336,12 @@ int sbus_invoke_get_aDOsasDE(DBusMessageIter *iter,
/* iterate over keys */
if (table == NULL) {
+ dbret = dbus_message_iter_close_container(iter, &it_array);
+ if (!dbret) {
+ ret = EIO;
+ goto done;
+ }
+
ret = EOK;
goto done;
}
New patches are attached.
I'm still testing functionality, but Coverity reported dead code:
Error: DEADCODE (CWE-561):
sssd-1.12.90/src/responder/ifp/ifp_groups.c:325: cond_notnull: Condition "req ==
NULL", taking false branch. Now the value of "req" is not
"NULL".
sssd-1.12.90/src/responder/ifp/ifp_groups.c:357: notnull: At condition "req ==
NULL", the value of "req" cannot be "NULL".
sssd-1.12.90/src/responder/ifp/ifp_groups.c:357: dead_error_condition: The condition
"req == NULL" cannot be true.
sssd-1.12.90/src/responder/ifp/ifp_groups.c:358: dead_error_begin: Execution cannot reach
this statement: "ret = 12;".
# 356| domain->name, name);
# 357| if (req == NULL) {
# 358|-> ret = ENOMEM;
# 359| goto immediately;
# 360| }
and ifp unit test is consistently failing for me:
FAIL: ifp_tests
===============
[==========] Running 6 test(s).
[ RUN ] ifp_test_req_create
[ OK ] ifp_test_req_create
[ RUN ] ifp_test_req_wrong_uid
[ OK ] ifp_test_req_wrong_uid
[ RUN ] test_el_to_dict
[ OK ] test_el_to_dict
[ RUN ] test_attr_acl
s2[i]
/home/remote/jhrozek/devel/sssd/src/tests/cmocka/test_ifp.c:214: error: Failure!
[ FAILED ] test_attr_acl
[ RUN ] test_attr_acl_ex
[ OK ] test_attr_acl_ex
[ RUN ] test_attr_allowed
[ OK ] test_attr_allowed
[==========] 6 test(s) run.
[ PASSED ] 5 test(s).
[ FAILED ] 1 test(s), listed below:
[ FAILED ] test_attr_acl
1 FAILED TEST(S)