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
next prev parent 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