Re: [evolution-patches] File descriptor leaks in e-d-s

[Jules Colding <colding omesc com>]

On Sat, 2007-05-12 at 12:58 -0400, Jeffrey Stedfast wrote:
> Yea, this looks good - thanks for the fix!

Committed.

Best regards,
  jules


> Jeff
> 
> On Sat, 2007-05-12 at 12:30 +0200, Jules Colding wrote:
> > On Fri, 2007-05-11 at 10:09 -0400, Jeffrey Stedfast wrote:
> > > On Fri, 2007-05-11 at 13:04 +0200, Jules Colding wrote:
> > > > Index: camel/providers/sendmail/camel-sendmail-transport.c
> > > > ===================================================================
> > > > --- camel/providers/sendmail/camel-sendmail-transport.c	(revision 7735)
> > > > +++ camel/providers/sendmail/camel-sendmail-transport.c	(working copy)
> > > > @@ -163,6 +163,8 @@
> > > >  	pid = fork ();
> > > >  	switch (pid) {
> > > >  	case -1:
> > > > +		close (fd[0]);
> > > > +		close (fd[1]);
> > > >  		camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
> > > >  				      _("Could not fork sendmail: "
> > > >  					"%s: mail not sent"),
> > > 
> > > you need to close the fds after setting the exception or errno will get
> > > cleared/won't refer to the fork() error.
> > 
> > Of cause - this and the other cases are oversights on my part. Sorry.
> > What about this patch instead?
> > 
> > 
> > Index: camel/providers/sendmail/camel-sendmail-transport.c
> > ===================================================================
> > --- camel/providers/sendmail/camel-sendmail-transport.c	(revision 7735)
> > +++ camel/providers/sendmail/camel-sendmail-transport.c	(working copy)
> > @@ -167,6 +167,8 @@
> >  				      _("Could not fork sendmail: "
> >  					"%s: mail not sent"),
> >  				      g_strerror (errno));
> > +		close (fd[0]);
> > +		close (fd[1]);
> >  		sigprocmask (SIG_SETMASK, &omask, NULL);
> >  		g_free (argv);
> >  		
> > Index: camel/providers/sendmail/ChangeLog
> > ===================================================================
> > --- camel/providers/sendmail/ChangeLog	(revision 7735)
> > +++ camel/providers/sendmail/ChangeLog	(working copy)
> > @@ -1,3 +1,7 @@
> > +2007-05-11  Jules Colding  <colding omesc com>
> > +
> > +	* camel-sendmail-transport.c (sendmail_send_to): Fix file descriptor leak
> > +
> >  2005-09-16  Tor Lillqvist  <tml novell com>
> >  
> >  	* camel-sendmail-transport.c: Use g_ascii_strcasecmp() instead of
> > @@ -14,4 +18,4 @@
> >  	* camel-sendmail-provider.c (camel_provider_module_init):
> >  	set translation_domain in CamelProvider struct.
> >  
> > -** refer to main changelog for earlier changes
> > \ No newline at end of file
> > +** refer to main changelog for earlier changes
> > Index: camel/ChangeLog
> > ===================================================================
> > --- camel/ChangeLog	(revision 7735)
> > +++ camel/ChangeLog	(working copy)
> > @@ -1,3 +1,9 @@
> > +2007-05-11  Jules Colding  <colding omesc com>
> > +
> > +	* camel-lock-client.c (camel_lock_helper_init): Fix file descriptor leak
> > +
> > +	* camel-process.c (camel_process_fork): Fix file descriptor leak
> > +
> >  2007-05-07  Matthew Barnes  <mbarnes redhat com>
> >  
> >  	* camel-stream-vfs.c:
> > Index: camel/camel-process.c
> > ===================================================================
> > --- camel/camel-process.c	(revision 7735)
> > +++ camel/camel-process.c	(working copy)
> > @@ -89,6 +89,8 @@
> >  		camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
> >  				      _("Failed to create child process '%s': %s"),
> >  				      argv[0], strerror (errno));
> > +		for (i = 0; i < 6; i++) 
> > +			close (fd[i]);
> >  		return -1;
> >  	}
> >  	
> > Index: camel/camel-lock-client.c
> > ===================================================================
> > --- camel/camel-lock-client.c	(revision 7735)
> > +++ camel/camel-lock-client.c	(working copy)
> > @@ -94,11 +94,24 @@
> >  {
> >  	int i;
> >  
> > +	lock_stdin_pipe[0] = -1;
> > +	lock_stdin_pipe[1] = -1;
> > +	lock_stdout_pipe[0] = -1;
> > +	lock_stdout_pipe[1] = -1;
> >  	if (pipe(lock_stdin_pipe) == -1
> >  	    || pipe(lock_stdout_pipe) == -1) {
> >  		camel_exception_setv (ex, CAMEL_EXCEPTION_SYSTEM,
> >  				      _("Cannot build locking helper pipe: %s"),
> >  				      g_strerror (errno));
> > +		if (lock_stdin_pipe[0] != -1)
> > +			close(lock_stdin_pipe[0]);
> > +		if (lock_stdin_pipe[1] != -1)
> > +			close(lock_stdin_pipe[1]);
> > +		if (lock_stdout_pipe[0] != -1)
> > +			close(lock_stdout_pipe[0]);
> > +		if (lock_stdout_pipe[1] != -1)
> > +			close(lock_stdout_pipe[1]);
> > +
> >  		return -1;
> >  	}
> >  
> > 
> >