On Wed, 04 Jan 2012 13:03:26 -0700 Eric Blake <ebl...@redhat.com> wrote:
> On 01/04/2012 12:45 PM, Luiz Capitulino wrote: > > + if (pid == 0) { > > + /* child */ > > + int fd; > > + > > + setsid(); > > + fclose(stdin); > > + fclose(stdout); > > + fclose(stderr); > > + > > + execlp(pmutils_bin, pmutils_bin, NULL); > > It's generally a bad idea to exec a child process without fd 0, 1, and 2 > open on something, even if that something is /dev/null. POSIX says that > the system may, but not must, reopen fds on your behalf, and that the > child without open std descriptors is then executing in a non-conforming > environment and may misbehave in unexpected manners. You're right. I just copied what we do in qmp_guest_shutdown()... I think we have to open /dev/null then, do you agree Michael? I think I'm doing to use dup2(), like dup2(dev_null_fd, 0). Any better recommendation? > > > + > > + /* > > + * The exec call should not return, if it does something went > > wrong. > > + * In this case we try to suspend manually if 'mode' is 'hibernate' > > + */ > > + slog("could not execute %s: %s\n", pmutils_bin, strerror(errno)); > > + slog("trying to suspend using the manual method...\n"); > > + > > + fd = open(LINUX_SYS_STATE_FILE, O_WRONLY); > > Worse, since you _just_ closed stdin above, fd here will most likely be > 0, but a O_WRONLY stdin is asking for problems. > > > + if (fd < 0) { > > + slog("can't open file %s: %s\n", LINUX_SYS_STATE_FILE, > > + strerror(errno)); > > Also, I have no idea where slog() writes to, but since you closed > stderr, if slog() is trying to use stderr, your error messages would be > invisible. >