Re: [Xen-devel] [PATCH v2] golang/xenlight: Add libxl_utils support

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Jun 28, 2019, at 9:01 PM, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
> 
> 
> 
>> On Jun 28, 2019, at 5:32 PM, George Dunlap <george.dunlap@xxxxxxxxxx> 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@xxxxxxxxx>
>> 
>> 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@xxxxxxxxx>
>> ---
>> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel