Re: [PATCH 1/4] golang/xenlight: add NameToDomid and DomidToName util functions

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Apr 22, 2020, at 8:46 PM, Nick Rosbrook <rosbrookn@xxxxxxxxx> wrote:
> 
>> libxl.h defines INVALID_DOMID — do we want to define an exported constant 
>> with the same name and use that here?  (Although part of me wonders if 
>> DOMID_INVALID would be a better option.)
> 
> Yeah, that makes sense. I'll add that.
> 
>>> +     }
>>> +
>>> +     return Domid(domid), nil
>>> +}
>>> +
>>> +// DomidToName returns the name for a domain, given its domid.
>>> +func (Ctx *Context) DomidToName(domid Domid) string {
>>> +     cname := C.libxl_domid_to_name(Ctx.ctx, C.uint32_t(domid))
>>> +     defer C.free(unsafe.Pointer(cname))
>>> +
>>> +     return C.GoString(cname)
>>> +}
>> 
>> It looks to me like if the domid doesn’t exist, libxl_domid_to_name() will 
>> return NULL; and then DomidToName will return “”.  Is that what we want?
>> 
>> If so, it should probably be documented.
> 
> I considered returning an error if C.GoString(cname) == "". But, with
> these functions (as well as the others in these series), I opted to
> keep the signatures aligned with their libxl counterparts since we're
> keeping the package API mostly one-to-one with libxl. I can add a
> second return value if you prefer, otherwise I'll just add a note in
> the comment.

OK — adding a note in the comment is fine.  I mainly wanted to make sure the 
question had actually been considered (although of course documenting that 
behavior is also important).

Thanks,
 -George