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

[Jeffrey Stedfast <fejj novell com>]

Yea, this looks good - thanks for the fix!

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;
>  	}
>  
> 
>