Hi
----- Original Message -----
> > Because that field is for peer closing. I can rename it peer_closed
> > perhaps? (peer->peer_closed...)
>
> You lost me here. Do you mean PipeInputStream::closed means that the
> peer PipeOutputStream has been closed? If that's what you mean, then it
> definitely should be peer_closed.
I don't mind, for me the field means the pipe is closing, either side. However, since it's internal to the pipe implementation and is only set by the peer, I guess it could be closed peer_closed too. That really doesn't change much of the meaning of this code anyway.
> > Yes, I am not sure what that means in practice. There are also default
> > _async fallback for other methods, this might be outdated comment?
>
> Maybe in some places, gio is only checking if one of the async method is
> implemented, and then directly calls random other async methods. Is this
> _async implementation strictly needed anyway?
Probably not strictly required. I remember I based the code on gmemoryoutputstream and though it was a good idea to close sync and avoid threads (there are no pending IO handling either in giopipe)
However, note that close_async() has a default implementation, so it doesn't change much the checking for async implementation when overriding it.
> > >> +static gssize
> > >> +pipe_output_stream_write (GOutputStream *stream,
> > >> + const void *buffer,
> > >> + gsize count,
> > >> + GCancellable *cancellable,
> > >> + GError **error)
> > >> +{
> > >> + PipeOutputStream *self = PIPE_OUTPUT_STREAM(stream);
> > >> + PipeInputStream *peer = self->peer;
> > >> +
> > >> + //g_debug("write %p :%"G_GSIZE_FORMAT, stream, count);
> > >> + if (g_output_stream_is_closed (stream) || self->closed) {
> > >> + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
> > >> + "Stream is already closed");
> > >> + return -1;
> > >> + }
> > >> +
> > >> + /* this abuses pollable stream, writing sync would likely lead to
> > >> + crashes, since the buffer pointer would become invalid, a
> > >> + generic solution would need a copy..
> > >> + */
> > >> + g_return_val_if_fail(self->buffer == buffer || self->buffer ==
> > >> NULL, -1);
> > >> + self->buffer = buffer;
> > >> + self->count = count;
> > >> +
> > >> + pipe_input_stream_check_source(self->peer);
> > >
> > > This call will trigger a sync call to pipe_input_stream_read() if I
> > > followed everything properly? A comment mentioning that might make the
> > > code easier to follow.
> >
> > It's not sync, it will "schedule peer source". As said in the comment,
> > this abuses pollable stream, since it keeps a pointer to the buffer
> > and assumes the function will be resumed with the same data (there are
> > preconditions checks for that)
>
> Yup, the comment was not crystal clear, bit more understandable after looking
> at the next patches...
>
> Christophe
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel