[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 7/8] Use posix_spawn to run external scripts

From: Pawel Kot
Subject: Re: [PATCH v2 7/8] Use posix_spawn to run external scripts
Date: Mon, 20 Jan 2020 13:07:35 +0100

On Mon, Jan 20, 2020 at 9:40 AM Ladislav Michl <address@hidden> wrote:
> On Mon, Jan 20, 2020 at 12:03:30AM +0100, Pawel Kot wrote:
> > Hi,
> >
> > On Tue, Dec 4, 2018 at 10:32 PM Ladislav Michl <address@hidden> wrote:
> > > posix_spawn specification dates back to last century and its
> > > implementation is mature enough in all systems we do support.
> > > Thus use it instead of current fork and exec in hope it will
> > > save us some resources.
> >
> > So I think this one does more than described. My understanding is that now
> > you can pass env variables into a script. Is that correct? And what was
> > wrong with traditional fork/exec approach? I mean in our particular case.
> > Did you run into oom in some setups?
> Even current implementation is able to set environment for a script and there
> is nothing wrong with traditional fork/exec approach, except this is one of
> the worst unix apis ever seen ;-) Also that FIXME in original code could be
> fixed using O_CLOEXEC where available.

.. but a classic one. :)

> So, I do not insist on this patch as it is, but having device_script as
> separate platform specific function is still usefull, just because later
> we can implement it using CreateProcess Win32 API function (therefore
> not requiring cygwin). Here it is done in single patch to indicate
> with posixscript.c name that it is expected to work on every POSIX.1-2001
> conformant system. It is expected CreateProcess version to be named
> as win32script.c :)

Fair enough. Let me give another look.

> > I've merged all other patches except this one (and one related) and devices
> > build refactor (which I do not like in this form) into github.
> Any particular issue with that except that one you mentioned earlier? I mean
> #else
> int fbusdku2usb_open(struct gn_statemachine *state)
> {
>         return -1;
> }
> ...etc, it header files? This one I did to improve modularity and it does its
> job pretty well without touching too much core code. Alternatively I can
> turn it into proper device plugin architecture. Is that your preferred way
> to go? (while doing that I would also add support for external even loops)

Yes, if we're going to change that let's do it in a proper way. Happy to discuss on IRC sometime.

> Seems good for me and works for me on Linux. I'll give it a try on Win32
> and MacOS over the week.

Seems to compile on Mac. Will give a whirl on Windows during next weekend.

Pawel Kot

reply via email to

[Prev in Thread] Current Thread [Next in Thread]