On 1/17/20 6:13 PM, Nick Rosbrook wrote:
>> // 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.
OK. I think it looks ugly, but there's certainly a signalling value to
having it really be empty.
>
>> + // ...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.
Ack.
>> // 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`?
Er, yes, indeed it should. This has gone through too many iterations...
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
