Re: [Xen-devel] [PATCH v3 7/8] golang/xenlight: Notify xenlight of SIGCHLD

[Nick Rosbrook <rosbrookn@xxxxxxxxx>]

>  // Context represents a libxl_ctx.
>  type Context struct {
> -       ctx    *C.libxl_ctx
> -       logger *C.xentoollog_logger_stdiostream
> +       ctx         *C.libxl_ctx
> +       logger      *C.xentoollog_logger_stdiostream
> +       sigchld     chan os.Signal
> +       sigchldDone chan bool

It's preferred to use `chan struct{}` for this pattern; it makes it
clear that the data sent over the channel has no meaning, and is only
intended to be used for synchronization.

> +       // ...and arrange to keep that promise.
> +       ctx.sigchld = make(chan os.Signal, 2)
> +       ctx.sigchldDone = make(chan bool, 1)
> +       signal.Notify(ctx.sigchld, syscall.SIGCHLD)
> +
> +       go sigchldHandler(ctx)

It could be useful to add a comment here that explains the lifetime of
this goroutine, i.e. it returns when the context is Close()'d.

>  // Close closes the Context.
>  func (ctx *Context) Close() error {
> +       // Tell our SIGCHLD notifier to shut down, and wait for it to exit
> +       // before we free the context.
> +       if ctx.sigchld == nil {

Shouldn't this be `if ctx.sigchld != nil`?

-NR

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