[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [dbus PATCH 5/9] util: rename function to follow libvirt naming rules



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.

> > Signed-off-by: Pavel Hrdina <phrdina 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

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]