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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs