Re: [Xen-devel] [OPW PATCH] tools/libxl/xl_cmdimpl.c : Calling init function

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Oct 16, 2014 at 04:13:30PM +0100, George Dunlap wrote:
[...]
> > ---
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index c734f79..5b4de28 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -5157,6 +5157,7 @@ static int sched_domain_get(libxl_scheduler sched, 
> > int domid,
> >  {
> >      int rc;
> >
> > +    libxl_domain_sched_params_init(scinfo);
> 
> While this will technically work, I think it's probably better if this
> is done closer to the place where it's allocated.  That way it's easy
> to see the pattern you expect:
>  [declare]
>  ...
>  [init]
>  ...
>  [dispose]
> 

Agreed. _init and _dispose need to stay in the same function as variable
declaration.

> (Even if you decided to coalesce all the calls into one place,  then
> you should have removed the _init() call in sched_rtds_domain_output()
> (which is redundant with this patch).)
> 

No, please don't. I asked Meng to construct his function like that,
which is consistent with the aforementioned pattern.  :-)

Wei.

> CC'ing the tools maintainers so they can contradict me if they have a
> different opinion...
> 
> Thanks!
>  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel