[Spice-devel] [linux-agent v1 3/5] After fork, use one expression per line

[Victor Toso]

Hi,

On Tue, Dec 18, 2018 at 12:33:20PM -0500, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > And use well defined macros for standard file descriptors.
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/vdagent/vdagent.c   | 8 ++++++--
> >  src/vdagentd/vdagentd.c | 8 ++++++--
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> > index f7c8b72..87dfa7c 100644
> > --- a/src/vdagent/vdagent.c
> > +++ b/src/vdagent/vdagent.c
> > @@ -294,9 +294,13 @@ static int daemonize(void)
> >      /* detach from terminal */
> >      switch (fork()) {
> >      case 0:
> > -        close(0); close(1); close(2);
> > +        close(STDIN_FILENO);
> > +        close(STDOUT_FILENO);
> > +        close(STDERR_FILENO);
> >          setsid();
> > -        x = open("/dev/null", O_RDWR); x = dup(x); x = dup(x);
> > +        x = open("/dev/null", O_RDWR);
> > +        x = dup(x);
> > +        x = dup(x);
> >          close(fd[0]);
> >          return fd[1];
> >      case -1:
> > diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> > index 99683da..a2f0946 100644
> > --- a/src/vdagentd/vdagentd.c
> > +++ b/src/vdagentd/vdagentd.c
> > @@ -949,9 +949,13 @@ static void daemonize(void)
> >      /* detach from terminal */
> >      switch (fork()) {
> >      case 0:
> > -        close(0); close(1); close(2);
> > +        close(STDIN_FILENO);
> > +        close(STDOUT_FILENO);
> > +        close(STDERR_FILENO);
> >          setsid();
> > -        x = open("/dev/null", O_RDWR); x = dup(x); x = dup(x);
> > +        x = open("/dev/null", O_RDWR);
> > +        x = dup(x);
> > +        x = dup(x);
> >          pidfile = fopen(pidfilename, "w");
> >          if (pidfile) {
> >              fprintf(pidfile, "%d\n", (int)getpid());
> 
> Sure,
> Acked-by: Frediano Ziglio <fziglio at redhat.com>

Thanks,

> Maybe would be also good to rename "x" to some more sensible name ?

Some more warnings from coverity in regards to not checking 'x'
value from open() and possible passing negative value to dup().

Clang warns on unused variable and/or not checking for errors.

In the end I kept 'x' and did not address any of above (...)

> OT: Looks like the 2 hunks are pretty similar, maybe factoring
> out a common function?

Or to rewrite it in rust :)

> Frediano

Cheers,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20181219/d8bc909f/attachment.sig>