From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gorcunov@gmail.com>
Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com
 [209.85.208.195])
 (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 4565D46970E
 for <tarantool-patches@dev.tarantool.org>;
 Thu,  9 Jan 2020 14:23:35 +0300 (MSK)
Received: by mail-lj1-f195.google.com with SMTP id h23so6795953ljc.8
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 09 Jan 2020 03:23:35 -0800 (PST)
Date: Thu, 9 Jan 2020 14:23:32 +0300
From: Cyrill Gorcunov <gorcunov@gmail.com>
Message-ID: <20200109112332.GD2436@uranus>
References: <20191217125420.20881-1-gorcunov@gmail.com>
 <20191217125420.20881-3-gorcunov@gmail.com>
 <20191226071441.GB6901@atlas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191226071441.GB6901@atlas>
Subject: Re: [Tarantool-patches] [PATCH v6 2/4] popen: Introduce a backend
	engine
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Konstantin Osipov <kostja.osipov@gmail.com>
Cc: tml <tarantool-patches@dev.tarantool.org>

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?