Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja.osipov@gmail.com>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: tml <tarantool-patches@dev.tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine
Date: Fri, 29 Nov 2019 08:52:14 +0300	[thread overview]
Message-ID: <20191129055214.GG15149@atlas> (raw)
In-Reply-To: <20191128204512.19732-2-gorcunov@gmail.com>

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
>  - environment variables are flushed to zero, should we provide
>    a way to adjust it (via options) or inherit it instead?

Yes. You can take a look at Python Popen api for inspiration.

>  - popen_kill always send SIGKILL, should not we provide a
>    portable way to customize signal sedning (say symbolic
>    names for signals and pass them here)?

Again, you could take a look at Python - they have kill()
and terminate(), which allows for platform-agnostics hard and soft 
termination. Sending Unix signals to processes can be done by signal()
system call, so you don't need to worry about providing an API
for it in Popen, as long as you expose the child pid in the api.

>  - for native mode we don't do additional processing of arguments
>    thus only plain name of elf executable will be working, we
>    should provide a way for argv explicit passing or
>    do analyze @command for arguments by hands;

>  - need to consider a case where we will be using piping for
>    descriptors (for example we might be writting into stdin
>    of a child from another pipe, for this sake we could use
>    splice() syscall which gonna be a way faster than copying
>    data inside kernel between process). Still the question
>    is -- do we really need it? Since we use interanal flags
>    in popen handle this should not be a big problem to extend
>    this interfaces.

Not in the first version for sure. 
>  	title_free(main_argc, main_argv);
>  
> +	popen_fini();

The convention is to use new/delete for functions which
allocate + initialize and destroy + deallocate an object.

create/destroy for functions which initialize/destroy an object
but do not handle memory management.

init/free for functions which initialize/destroy libraries and
subsystems. Please stick to it.

-- 
Konstantin Osipov, Moscow, Russia

  reply	other threads:[~2019-11-29  9:09 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-28 20:45 [Tarantool-patches] [PATCH 0/5] popen: Add ability to run external process Cyrill Gorcunov
2019-11-28 20:45 ` [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine Cyrill Gorcunov
2019-11-29  5:52   ` Konstantin Osipov [this message]
2019-11-29  9:57     ` Cyrill Gorcunov
2019-11-29  5:59   ` Konstantin Osipov
2019-11-29  9:40     ` Cyrill Gorcunov
2019-11-29 11:19       ` Konstantin Osipov
2019-11-29 11:36         ` Cyrill Gorcunov
2019-11-29 14:50           ` Konstantin Osipov
2019-11-29 15:14             ` Cyrill Gorcunov
2019-11-29 15:17               ` Cyrill Gorcunov
2019-11-29 18:31               ` Konstantin Osipov
2019-11-29 19:17                 ` Cyrill Gorcunov
2019-11-29 22:36                   ` Cyrill Gorcunov
2019-11-30  4:21                     ` Konstantin Osipov
2019-11-30  7:48                       ` Cyrill Gorcunov
2019-11-30  4:14                   ` Konstantin Osipov
2019-11-30  7:36                     ` Cyrill Gorcunov
2019-11-30 10:04                       ` Konstantin Osipov
2019-11-30 10:47                         ` Cyrill Gorcunov
2019-11-30 10:54                           ` Cyrill Gorcunov
2019-11-30 12:16                             ` Cyrill Gorcunov
2019-11-30 20:30                             ` Konstantin Osipov
2019-11-30 20:36                               ` Cyrill Gorcunov
2019-12-13  2:50   ` Alexander Turenko
2019-11-28 20:45 ` [Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper Cyrill Gorcunov
2019-11-29  6:02   ` Konstantin Osipov
2019-11-29  9:47     ` Cyrill Gorcunov
2019-11-29 11:22       ` Konstantin Osipov
2019-11-29 11:42         ` Cyrill Gorcunov
2019-11-29 14:51           ` Konstantin Osipov
2019-11-28 20:45 ` [Tarantool-patches] [PATCH 3/5] popen/fio: Merge popen engine into fio internal module Cyrill Gorcunov
2019-11-28 20:45 ` [Tarantool-patches] [PATCH 4/5] popen/fio: Implement lua interface for a popen object Cyrill Gorcunov
2019-11-28 20:45 ` [Tarantool-patches] [PATCH 5/5] test: Add app/popen test Cyrill Gorcunov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191129055214.GG15149@atlas \
    --to=kostja.osipov@gmail.com \
    --cc=gorcunov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox