Re: [libvirt] [dbus PATCH 5/9] util: rename function to follow libvirt naming rules
On 08/02/2017 04:35 AM, Pavel Hrdina wrote:
On Tue, Aug 01, 2017 at 09:31:40AM -0400, John Ferlan wrote:
>
>
> On 07/24/2017 09:38 AM, Pavel Hrdina wrote:
>> All libvirt-dbus function should use virtDBus preffix and use only one
>> coding style, camelCase.
>>
>
> s/preffix/prefix
>
> In general... Should "bus_path" just be Path - seems redundant to have
> "virtDBusBus"... It's not wrong, but mainly curious especially since
the
> "bus_" prefix got changed to virtDBus.
I would like to keep the BusPath as a one unit, path can be something
generic and this operates with dbus path that specifies objects.
> FWIW: Might also have been easier to convert all those
> domain_from_bus_path calls into a helper first...
>
> Since manager.c gets "virtDBusManager", domain.c gets
"virtDBusDomain",
> and events.c gets "virtDBusEvents" should these be virtDBusUtil to
> distinguish them from main.c which also uses "virtDBus"?
I was considering it and I'll change it, it will follow the naming
convention closely.
I'm fine with keeping BusPath together - it was a suggestion, not a
requirement. As for virtDBusUtil - I think it's the better way to go -
if only to help figure out where to find code! Nothing in main.c is
external (yet), so you could get away without doing it too. Still better
to get it now than later too.
John
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/domain.c | 110 +++++++++++++++++++++++++++++-----------------------------
>> src/events.c | 20 +++++------
>> src/main.c | 2 +-
>> src/manager.c | 32 ++++++++---------
>> src/util.c | 17 ++++-----
>> src/util.h | 26 +++++++-------
>> 6 files changed, 105 insertions(+), 102 deletions(-)
>>
>> diff --git a/src/domain.c b/src/domain.c
>> index 1bda3b8..4bfb314 100644
>> --- a/src/domain.c
>> +++ b/src/domain.c
>
> [...]
>
>> @@ -236,13 +236,13 @@ domain_get_xml_desc(sd_bus_message *message,
>> sd_bus_error *error)
>> {
>> VirtManager *manager = userdata;
>> - _cleanup_(virDomainFreep) virDomainPtr domain = NULL;
>> - _cleanup_(freep) char *description = NULL;
>> + _cleanup_(virtDBusVirDomainFreep) virDomainPtr domain = NULL;
>> + _cleanup_(virtDBusFreep) char *description = NULL;
>> uint32_t flags;
>> int r;
>>
>> - domain = domain_from_bus_path(manager->connection,
>> - sd_bus_message_get_path(message));
>> + domain = virtDBusVirDomainFromBusPath(manager->connection,
>> + sd_bus_message_get_path(message));
>
> adjust alignment of 2nd argument (occurs a few more times too -
> domain_get_stats, domain_shutdown, domain_destroy, domain_reboot,
> domain_reset, domain_create, and domain_undefine)
Nice catch, I've actually noticed that while working on implementing
new APIs, now I know in which patch I've introduced it. I'll fix it.
Pavel