Re: [Nmh-workers] outstanding patches

[Harald Geyer]

> 
>   | Ok, then this should be changed too, in case anybody wants to keep
>   | vfork().
> 
> Yes, that ought be changed, though in practice the one in question
> should not really matter a lot, as it only happens if the exec fails,
> which should ordinarily not happen.
> 
> But, aside from the fprintf, ...
> 
>   | But I still think it's better to switch to fork() - nmh doesn't
>   | obey the rules necessary to use vfork() even if it isn't that bad as
>   | it seemed to me in the beginning.
> 
> I saw nothing in replsbr.c's use of vfork() (the bug referred to in the
> Debian bug report) that was incorrect - all that happens, assuming the
> exec succeeds, is that file descriptors are dup'd and closed.   That
> kind of thing is exactly what is normally planned to happen between
> vfork() and exec (and it is precisely because it is quite hard to
> specify exactly what should be done, in any way other than C code, that
> makes spawn() kind of difficult).

Ok, it's just my lack of knowledge, that I suggested to get rid of the
issue once and forever. If the code is ok, than I don't mind whether 
vfork() is kept or not.

> On the other hand, I also saw nothing there that would actually print
> an error message on file system full (the trigger condition for the
> bug report).   There's certainly an fflush() that could fail, and perhaps
> cause errno to get set, which the code in the vfork() case could
> then alter - but if nmh is expecting errno to remain unchanged from
> that fflush() (just before the vfork()) to some undisclosed later
> place, presumably where a ferror() test is done, and an error printed),
> then nmh is entirely deluded. 

Yes it is. BTW the error message is printed just after the only call
to replfilter().

> Aside from the vfork() the parent code
> also calls pidXwait() (which is pitstatus(pidwait(...)) and fseek()
> after the fflush before replfilter() ends (and the error message must
> occur somewhere later).   pidwait() does waitpid() (which might alter errno).
> and sometimes, signal() (which also might alter errno), pidstatus()
> can call fprintf() ...    After all of that, expecting errno to be
> unchanged is just plain wrong.

Yes I already realized that there are two bugs involved: Expecting errno
to remain unchanged and dealing with vfork()/fprintf().

> Changing vfork() into fork() may just happen to remove one cause of
> errno being altered altered, but if none other happens, that's pure fluke.

Of course you are right. The reason that it appeard to fix the bug is,
that the things you listed might change errno - the call of closefds()
will always do so.

Harald