On Wed, Jan 21, 2015 at 02:08:54PM +0100, Pavel Březina wrote:
2/3 of previous sbus changes are acked so I'm sending few more
patches from
my tree.
Commit messages should be sufficient to justify the changes. Simply put, I
wanted to standardize and shorten names of generated and hard coded
variables and constants so the code stays simple with longer interface
names.
From 610ffa5ee147f383ad2f876b6659bb9b92628808 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Thu, 15 Jan 2015 14:23:19 +0100
Subject: [PATCH 1/7] IFP: move interface definitions from ifpsrv.c into
separate file
Number of IFP interfaces will grown up rapidly in the future. It is
not convenient to keep it inside ifpsrv.c.
One little nitpick
+errno_t ifp_register_sbus_interface(struct sbus_connection *conn,
void *pvt)
Can you s/pvt/handler_data/ to be consistent with the changes we did in
sbus?
+{
+ errno_t ret;
+ int i;
+
+ for (i = 0; iface_map[i].path != NULL; i++) {
+ ret = sbus_conn_register_iface(conn, iface_map[i].vtable,
+ iface_map[i].path, pvt);
+ if (ret != EOK) {
+ return ret;
+ }
+ }
+
+ return EOK;
+}
From f5efe4b4ac3903d9c31bffe14c305eb2a23d56cf Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Thu, 15 Jan 2015 14:38:39 +0100
Subject: [PATCH 2/7] IFP: unify generated interfaces names
Number of interfaces will grow. It is mandatory to unify names of
generated structures and methods to simplify coding and debugging.
The C name is created from D-Bus lowercased interface name using
the following rewrite rules:
org.freedesktop.sssd.infopipe -> iface_ifp
. -> _
Example:
org.freedesktop.sssd.infopipe.Domains -> iface_ifp_domains
Again, nitpick:
#include <dbus/dbus.h>
#include "sbus/sssd_dbus.h"
-#include "monitor/monitor_interfaces.h"
-#include "confdb/confdb.h"
-#include "responder/ifp/ifp_private.h"
+#include "responder/ifp/ifp_iface_generated.h"
#include "responder/ifp/ifp_domains.h"
#include "responder/ifp/ifp_components.h"
-#include "responder/common/responder_sbus.h"
The include changes seem unrelated, or are they? (except adding
responder/ifp/ifp_iface_generated.h maybe)
From 0cad96634dffa09cabc9fb82ac2b760312696da1 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Thu, 15 Jan 2015 14:49:25 +0100
Subject: [PATCH 3/7] sbus codegen: do not prefix getters with iface name
Prefixing getters with C name of the interface is just
redundant since it is the same as the name of the structure
that contains those fields.
The following structure:
struct test_pilot {
$type test_pilot_get_name;
}
changes to:
struct test_pilot {
$type get_name;
}
---
I'm quite indifferent to this change, but if you feel it would make the
code more maintainable, ACK
From e816d0414b49d43cfe58920793f9106c567ab6aa Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Fri, 16 Jan 2015 13:36:12 +0100
Subject: [PATCH 4/7] IFP: simplify object path constant names
The number of interfaces will grow and in order to keep names of
object path constant consistent it means that also their names
will grow. The new naming schema is:
/org/freedesktop/sssd/infopipe -> IFP_PATH_
/ -> _
everything uppercase
Example:
/org/freedesktop/sssd/infopipe/Components/Responders
-> IFP_PATH_COMPONENTS_RESPONDERS
If the path contains a _TREE suffix, it represents the whole
subtree. For example:
IFP_PATH_DOMAINS is /org/freedesktop/sssd/infopipe/Domains/*
Ack to the code, but didn't you mean to say IFP_PATH_DOMAINS_TREE in the
commit message? (I like the _TREE change btw)
From b959cfa030153b85b9dd7c355ba9bdd89b7ab4a7 Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Wed, 21 Jan 2015 13:47:28 +0100
Subject: [PATCH 5/7] sbus: add constant to represent subtree
ACK