Hello!
> This will break the native Windows build, so please don't. (See job.h
> for how this macro is defined on Windows.) And even if the places
> where you made those expansions are not compiled on Windows, having a
> macro in some places and its expansion in others is confusing.
Ok, i actually can leave it as a macro. I decided to expand it for better
build reliability. I imagined that in some weird situations (broken include
files for example, broken compiler, whatever) there will be no compile
error, but the resulting executable will leak fd's. So i decided to make
sure that FD_CLOEXEC is really set. Yes, this part of code really does not
compile on Windows.
But if you dislike it, of course it's possible to keep a macro.
> Now systems other than EMX that use 'fork/exec' will not declare
> child_execute_job as a function that does not return, which is needed,
> since the call to 'exec_command' doesn't return.
But child_execute_job() does return, because now fork() is moved inside it.
It returns in parent context. To be more clear, here is algorithmic
difference:
Old version: fork(), then (in child's context) manually close() unneeded
fd's, then jump to child_execute_job() which does not return.
New version: set CLOEXEC flag for unneeded fd's then call
child_execute_job(). It fork()s inside, and returns in parent context.
As a result, we now have this fork() in a single place (instead of two),
and it is much easier to introduce a runtime switch between fork() and
spawn() now.
> And what about this FIXME comment:
>
> > + /* undo FD_CLOEXEC after the child process has been started
> > + CHECKME: Is this really needed ? These flags should not
> > + harm. */
> > if (!(flags & COMMANDS_RECURSE) && job_fds[0] >= 0)
> > {
It's there to attract other's attention (it did the job :)) and gather some
opinions. What do you think, may be this is really redundant to reset
CLOEXEC after forking ? I don't see any situations where these fd's should
be passed on to child process. Every time we run a child, we use only
redirection of stdin/stdout/stderr. We don't run any special children which
would expect more fd's to be there.
> Thanks again for working on this.
Thank you for your appreciation.
By the way, i have two systems of Amiga family (MorphOS and AROS), are you
interested in testing and some face-lifting to Amiga-specific code ?
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
_______________________________________________
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make
