On Thu, Apr 14, 2016 at 08:04:39AM -0600, Eric Blake wrote:
On 04/14/2016 07:57 AM, Richard W.M. Jones wrote:
On Thu, Apr 14, 2016 at 07:38:23AM -0600, Eric Blake wrote:
+ /* Read the umask. */ + if (read (fd[0], &mask, sizeof mask) != sizeof mask) { + perrorf (g, "read"); + close (fd[0]); + return -1;
Oops - this strands a child process. You have to reap the child, even if the read() failed.
Bleah that was stupid. Try attached version - the only difference is I added a waitpid call to the error path above.
But without looping on EINTR...
Thanks - I pushed it with your suggested loop added. BTW we don't loop in waitpid/EINTR anywhere else in libguestfs, but I guess this is something we should be doing. Can you tell us why it's necessary? Rich.
+ + /* Read the umask. */ + if (read (fd[0], &mask, sizeof mask) != sizeof mask) { + perrorf (g, "read"); + close (fd[0]); + waitpid (pid, NULL, 0);
I think it's enough to make this line:
while (waitpid (pid, &status, 0) == -1 && errno == EINTR);
+ return -1;
as you've already reported the initial error, and are merely focused on cleanup of the child.
+ } + close (fd[0]); + + again: + if (waitpid (pid, &status, 0) == -1) { + if (errno == EINTR) goto again; + perrorf (g, "waitpid"); + return -1;
Anything else, like trying to reuse the again: loop, seems like it be more complicated control flow.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW