[SSSD] [PATCH] DBus: Refactor how requests are handled
Pavel Březina
pbrezina at redhat.com
Fri Mar 7 13:55:07 UTC 2014
On 02/28/2014 06:15 PM, Stef Walter wrote:
> On 24.02.2014 18:50, Stef Walter wrote:
>> On 24.02.2014 16:23, Lukas Slebodnik wrote:
>>> On (24/02/14 11:42), Stef Walter wrote:
>>>> Here's the next patchset for refactoring the DBus support in sssd.
>>>>
>>> I didn't test patches. I am only sending warnings from compiler and
>>> static analysers.
>>
>> Thanks. Attached are the fixed patches.
>
> Rebased on master, and tweaked sbus_request_fail_and_finish() to better
> accomodate code in the dbus-invoke branch.
>
> Stef
Hi,
really great job! I have several comments, mostly coding style issues.
Patch 1: Ack.
Patch 2:
- can you use either dbus_req or request as sbus_request name? For
example dp handlers have 'request' in prototype, but 'dbus_req' in
declaration.
- we usually use 'ret' instead of 'res' to indicate variable containing
return value. And lowercased FALSE/TRUE. But that's no biggie.
- we use always brackets around if, if...else statements. Even for one
liners. Especially these types are hard to read.
+ if (!intf)
+ ret = ENOMEM;
+ else
+ ret = sbus_conn_add_interface(ctx->conn, intf);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_FATAL_FAILURE, "Failed to export proxy.\n");
+ return ret;
+ }
+ if (!sbus_request_parse_or_finish(dbus_req,
+ DBUS_TYPE_UINT32, &type,
+ DBUS_TYPE_UINT32, &attr_type,
+ DBUS_TYPE_STRING, &filter,
+ DBUS_TYPE_STRING, &domain,
+ DBUS_TYPE_INVALID))
+ return EOK; /* handled */
Those coding style issues are valid also for the third patch.
Patch 3:
- prototype of dp handlers contains 'data', declarations have 'user_data'
- coding style issues, see #2
Patch 4: Ack.
Patch 5:
- coding style issues, see #2
Acks are code-wise only so far. It would be nice if someone else
reviewed it as well, since it is large patch set and most of the changes
are pattern replacement, it is highly possible I have missed something.
More information about the sssd-devel
mailing list