| Message ID | 20190628082508.32509-1-nicolas.belouin@gandi.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] golang/xenlight: Add libxl_utils support | expand |
On 6/28/19 9:25 AM, Nicolas Belouin wrote: > The Go bindings for libxl miss functions from libxl_utils, let's start > with the simple libxl_domid_to_name and its counterpart > libxl_name_to_domid. > > Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> Just for future reference, below your SoB, it's good practice to put a `---` line (below which everything will be ignored), and a list of the changes you made. E.g,: Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> --- v2: - Don't leak C string returned by libxl_domid_to_name One more thing... > +//char* libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid); > +func (Ctx *Context) DomidToName(id Domid) (name string) { > + cDomName := C.libxl_domid_to_name(Ctx.ctx, C.uint32_t(id)) > + defer C.free(unsafe.Pointer(cDomName)) > + > + name = C.GoString(cDomName) libxl_domid_to_name() returns NULL if domid doesn't exist. Will this code DTRT (returning 'nil' in that case)? Or will it crash / do something else? I couldn't actually find the answer in a quick search for the documentation. Any chance you could build a test program to see what happens? Alternately, we could play it safe and always check cDomName for `nil` before passing it to C.GoString(). Thanks, -George
> On Jun 28, 2019, at 5:32 PM, George Dunlap <george.dunlap@citrix.com> wrote: > > On 6/28/19 9:25 AM, Nicolas Belouin wrote: >> The Go bindings for libxl miss functions from libxl_utils, let's start >> with the simple libxl_domid_to_name and its counterpart >> libxl_name_to_domid. >> >> Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> > > Just for future reference, below your SoB, it's good practice to put a > `---` line (below which everything will be ignored), and a list of the > changes you made. E.g,: > > Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> > --- > v2: > - Don't leak C string returned by libxl_domid_to_name > > One more thing... > >> +//char* libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid); >> +func (Ctx *Context) DomidToName(id Domid) (name string) { >> + cDomName := C.libxl_domid_to_name(Ctx.ctx, C.uint32_t(id)) >> + defer C.free(unsafe.Pointer(cDomName)) >> + >> + name = C.GoString(cDomName) > > libxl_domid_to_name() returns NULL if domid doesn't exist. Will this > code DTRT (returning 'nil' in that case)? Or will it crash / do > something else? > > I couldn't actually find the answer in a quick search for the > documentation. Any chance you could build a test program to see what > happens? > > Alternately, we could play it safe and always check cDomName for `nil` > before passing it to C.GoString(). I just asked, and it turns out if C.GoString() is passed a nil pointer, it returns the empty string (“”), which is what we want. It’s not documented yet, but there’s a ticket to document it soon. https://github.com/golang/go/issues/32734 So this is ready to go in: Reviewed-by: George Dunlap <george.dunlap@citrix.com>
> On Jun 28, 2019, at 9:01 PM, George Dunlap <george.dunlap@citrix.com> wrote: > > > >> On Jun 28, 2019, at 5:32 PM, George Dunlap <george.dunlap@citrix.com> wrote: >> >> On 6/28/19 9:25 AM, Nicolas Belouin wrote: >>> The Go bindings for libxl miss functions from libxl_utils, let's start >>> with the simple libxl_domid_to_name and its counterpart >>> libxl_name_to_domid. >>> >>> Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> >> >> Just for future reference, below your SoB, it's good practice to put a >> `---` line (below which everything will be ignored), and a list of the >> changes you made. E.g,: >> >> Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> >> --- >> v2: >> - Don't leak C string returned by libxl_domid_to_name >> >> One more thing... >> >>> +//char* libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid); >>> +func (Ctx *Context) DomidToName(id Domid) (name string) { >>> + cDomName := C.libxl_domid_to_name(Ctx.ctx, C.uint32_t(id)) >>> + defer C.free(unsafe.Pointer(cDomName)) >>> + >>> + name = C.GoString(cDomName) >> >> libxl_domid_to_name() returns NULL if domid doesn't exist. Will this >> code DTRT (returning 'nil' in that case)? Or will it crash / do >> something else? >> >> I couldn't actually find the answer in a quick search for the >> documentation. Any chance you could build a test program to see what >> happens? >> >> Alternately, we could play it safe and always check cDomName for `nil` >> before passing it to C.GoString(). > > I just asked, and it turns out if C.GoString() is passed a nil pointer, it returns the empty string (“”), which is what we want. It’s not documented yet, but there’s a ticket to document it soon. > > https://github.com/golang/go/issues/32734 > > So this is ready to go in: Actually, turns out it’s not: You added a file, but it’s not wired into the build system. You need to add xenlight_utils.go to PKGSOURCES in the Makefile. I’ve just sent an updated patch. -George
diff --git a/tools/golang/xenlight/xenlight_utils.go b/tools/golang/xenlight/xenlight_utils.go new file mode 100644 index 0000000000..da1636842d --- /dev/null +++ b/tools/golang/xenlight/xenlight_utils.go @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2019 Nicolas Belouin, Gandi SAS + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see <http://www.gnu.org/licenses/>. + */ +package xenlight + +/* +#cgo LDFLAGS: -lxenlight -lyajl -lxentoollog +#include <stdlib.h> +#include <libxl_utils.h> +*/ +import "C" + +import ( + "unsafe" +) + +//char* libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid); +func (Ctx *Context) DomidToName(id Domid) (name string) { + cDomName := C.libxl_domid_to_name(Ctx.ctx, C.uint32_t(id)) + defer C.free(unsafe.Pointer(cDomName)) + + name = C.GoString(cDomName) + return +} + +//int libxl_name_to_domid(libxl_ct *ctx, const char *name, uint32_t *domid) +func (Ctx *Context) NameToDomid(name string) (id Domid, err error) { + cname := C.CString(name) + defer C.free(unsafe.Pointer(cname)) + + var cDomId C.uint32_t + + ret := C.libxl_name_to_domid(Ctx.ctx, cname, &cDomId) + if ret != 0 { + err = Error(-ret) + return + } + + id = Domid(cDomId) + + return +}
The Go bindings for libxl miss functions from libxl_utils, let's start with the simple libxl_domid_to_name and its counterpart libxl_name_to_domid. Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net> --- tools/golang/xenlight/xenlight_utils.go | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 tools/golang/xenlight/xenlight_utils.go