nmh-workers
[Top] [All Lists]

Re: [Nmh-workers] outstanding patches

2005-05-27 09:01:56

  | 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



_______________________________________________
Nmh-workers mailing list
Nmh-workers(_at_)nongnu(_dot_)org
http://lists.nongnu.org/mailman/listinfo/nmh-workers

<Prev in Thread] Current Thread [Next in Thread>