[PATCHES] few improvements to sbus and ifp

[Pavel Březina]

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.

[Jakub Hrozek]

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?= 
> 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?= 
> 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?= 
> 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?= 
> 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?= 
> Date: Wed, 21 Jan 2015 13:47:28 +0100
> Subject: [PATCH 5/7] sbus: add constant to represent subtree

ACK

[Pavel Březina]

On 02/11/2015 08:23 PM, Jakub Hrozek wrote:
> 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?= 
> > 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?

Done.
> > From f5efe4b4ac3903d9c31bffe14c305eb2a23d56cf Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > 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)

This belongs to the first patch since it removes headers that are no 
longer needed when the code was moved. I moved it to the first patch.
> > From e816d0414b49d43cfe58920793f9106c567ab6aa Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > 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)

You are correct sir.

New patches are attached, thank you for the review.

[Jakub Hrozek]

On Thu, Feb 12, 2015 at 11:38:06AM +0100, Pavel Březina wrote:
> New patches are attached, thank you for the review.

All my comments were addressed and a smoke test of IFP still passes.

ACK

[Jakub Hrozek]

On Mon, Feb 16, 2015 at 06:02:20PM +0100, Jakub Hrozek wrote:
> On Thu, Feb 12, 2015 at 11:38:06AM +0100, Pavel Březina wrote:
> > New patches are attached, thank you for the review.
>
> All my comments were addressed and a smoke test of IFP still passes.
>
> ACK

Obligatory CI link:
http://sssd-ci.duckdns.org/logs/job/7/45/summary.html

[Jakub Hrozek]

On Mon, Feb 16, 2015 at 06:02:20PM +0100, Jakub Hrozek wrote:
> On Thu, Feb 12, 2015 at 11:38:06AM +0100, Pavel Březina wrote:
> > New patches are attached, thank you for the review.
>
> All my comments were addressed and a smoke test of IFP still passes.
>
> ACK

* e3a7f7ee06cf0764838c45419bed97eb4cdf00f1
* 62ebed8582285bd24efba92b9a06366511507946
* 4e5d19f659d8c545c4ed3c307c95cfe4f2ca33cb
* beeef7f627a5ed9264de25ee4c76eb9620c1c984
* 772199031f0ec687fa1fefd939206858c440e5a1