Re: [Tarantool-patches] [PATCH v6 2/4] popen: Introduce a backend engine

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Thu, Dec 26, 2019 at 10:14:41AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/12/17 15:57]:
> > +ssize_t
> > +popen_read_timeout(struct popen_handle *handle, void *buf,
> > +		   size_t count, unsigned int flags,
> > +		   ev_tstamp timeout)
> > +{
> > +	int idx = flags & POPEN_FLAG_FD_STDOUT ?
> > +		STDOUT_FILENO : STDERR_FILENO;
> > +
> > +	if (!popen_may_io(handle, idx, flags))
> > +		return -1;
> > +
> > +	if (count > (size_t)SSIZE_MAX) {
> > +		errno = E2BIG;
> > +		return -1;
> > +	}
> > +
> > +	if (timeout < 0.)
> > +		timeout = TIMEOUT_INFINITY;
> > +
> > +	say_debug("popen: %d: read idx [%s:%d] buf %p count %zu "
> > +		  "fds %d timeout %.9g",
> > +		  handle->pid, stdX_str(idx), idx, buf, count,
> > +		  handle->fds[idx], timeout);
> > +
> > +	return coio_read_fd_timeout(handle->fds[idx],
> > +				    buf, count, timeout);
> > +}
> 
> Right, so could you please use struct coio here, and not
> coio_read_fd_timeout? I.e. convert handle->fds to coio?

Kostya, could you please elaborate this moment, since I
suspect I'm missing something.

1) coio service data, such as coio_wdata, coio_wait_cb is
   static to coio.cc file, you propose to make it exportable
   to other parts of tarantool code?

2) the case where timeout is really needed is when pipes
   are either empty (thus we will wait with timout until
   data appear, and adding fd into event loop doesn't look
   like a hot path) or full (where we will wait for data
   to be read first by another peer).

3) assuming we somehow exported data from (1) I don't really
   see how could we code the way we would put fd into backend
   once for all timelife of the popen object. Could you point
   please?