[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/5] golang/xenlight: Add host-related functionality
On Fri, Mar 03, 2017 at 02:54:56PM +0000, George Dunlap wrote: > On 02/03/17 16:07, Ronald Rojas wrote: > > Add calls for the following host-related functionality: > > - libxl_get_max_cpus > > - libxl_get_online_cpus > > - libxl_get_max_nodes > > - libxl_get_free_memory > > - libxl_get_physinfo > > - libxl_get_version_info > > > > Include Golang versions of the following structs: > > - libxl_physinfo as Physinfo > > - libxl_version_info as VersionInfo > > - libxl_hwcap as Hwcap > > > > Signed-off-by: Ronald Rojas <ronladred@xxxxxxxxx> > > Looks good -- just two minor comments... > > > --- > > Changes since last version > > > > - Refactored creating Physinfo and VersionInfo types into > > seperate toGo() functions. > > > > - Used libxl_<type>_init() and libxl_<type>_dispose() on IDL > > type physinfo > > > > - Whitespace fixes > > > > CC: xen-devel@xxxxxxxxxxxxx > > CC: george.dunlap@xxxxxxxxxx > > CC: ian.jackson@xxxxxxxxxxxxx > > CC: wei.liu2@xxxxxxxxxx > > --- > > --- > > tools/golang/xenlight/xenlight.go | 200 > > ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 200 insertions(+) > > > > diff --git a/tools/golang/xenlight/xenlight.go > > b/tools/golang/xenlight/xenlight.go > > index cbd3527..63cc805 100644 > > --- a/tools/golang/xenlight/xenlight.go > > +++ b/tools/golang/xenlight/xenlight.go > > @@ -106,6 +106,103 @@ type Context struct { > > ctx *C.libxl_ctx > > } > > > > +type Hwcap []C.uint32_t > > + > > +func (chwcap C.libxl_hwcap) CToGo() (ghwcap Hwcap) { > > Was there a reason you left this as CToGo() rather than just toGo()? > No. This should be changed to toGo() [snip] > > +//int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo) > > +func (Ctx *Context) GetPhysinfo() (physinfo *Physinfo, err error) { > > + err = Ctx.CheckOpen() > > + if err != nil { > > + return > > + } > > + var cphys C.libxl_physinfo > > + C.libxl_physinfo_init(&cphys) > > + > > + ret := C.libxl_get_physinfo(Ctx.ctx, &cphys) > > + > > + if ret < 0 { > > + err = Error(ret) > > + return > > + } > > + physinfo = cphys.toGo() > > + C.libxl_physinfo_dispose(&cphys) > > I think it would probably be more idiomatic to write "defer > C.libxl_physinfo_dispose()" just after the physinfo_init. > > If the init() actually allocated any memory, the current code would > return without disposing of it if there was an error. `defer` avoids > that by ensuring that *all* return paths call the clean-up function. Yep. Using defer is the better approach here. I will change it in all instances where I use the init/dispose functions. > > Other than that, looks great, thanks! > > -George > Thanks, Ronald _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |