Re: [Xen-devel] [PATCH v3 21/22] golang/xenlight: revise use of Context type

From: George Dunlap <george.dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>, <xen-devel@lists.xenproject.org>
Cc: Nick Rosbrook <rosbrookn@ainfosec.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	kerriganb@ainfosec.com, Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH v3 21/22] golang/xenlight: revise use of Context type
Date: Tue, 17 Dec 2019 17:28:05 +0000	[thread overview]
Message-ID: <5b92c6b7-e515-0eae-8722-de91cfade9b7@citrix.com> (raw)
In-Reply-To: <8330ea427861ef6c5d3b20d381bf40e87937d448.1575990937.git.rosbrookn@ainfosec.com>

On 12/10/19 3:47 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook <rosbrookn@ainfosec.com>
> 
> Remove the exported global context variable, 'Ctx.' Generally, it is
> better to not export global variables for use through a Go package.
> However, there are some exceptions that can be found in the standard
> library.
> 
> Add a NewContext function instead, and remove the Open, IsOpen, and
> CheckOpen functions as a result.
> 
> Also, comment-out an ineffectual assignment to 'err' inside the function
> Context.CpupoolInfo so that compilation does not fail.
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

With one requested change...

> ---
>  tools/golang/xenlight/xenlight.go | 219 +++++-------------------------
>  1 file changed, 34 insertions(+), 185 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index f32eb11384..1c431fa4e5 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -74,6 +74,39 @@ func (e Error) Error() string {
>  	return fmt.Sprintf("libxl error: %d", -e)
>  }
>  
> +// Context represents a libxl_ctx.
> +type Context struct {
> +	ctx    *C.libxl_ctx
> +	logger *C.xentoollog_logger_stdiostream
> +}
> +
> +// NewContext returns a new Context.
> +func NewContext() (*Context, error) {
> +	var ctx Context
> +
> +	ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
> +
> +	ret := C.libxl_ctx_alloc(&ctx.ctx, C.LIBXL_VERSION, 0, (*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))

This line looks to be 114 characters long, which seems a bit much. :-)
Mind breaking it just before the last argument?

Thanks,
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel