You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jf...@apache.org on 2010/02/16 18:07:10 UTC
svn commit: r910597 - /apr/apr/trunk/shmem/unix/shm.c
Author: jfclere
Date: Tue Feb 16 17:07:09 2010
New Revision: 910597
URL: http://svn.apache.org/viewvc?rev=910597&view=rev
Log:
Make sure we don't leak file descriptors.
Modified:
apr/apr/trunk/shmem/unix/shm.c
Modified: apr/apr/trunk/shmem/unix/shm.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/shmem/unix/shm.c?rev=910597&r1=910596&r2=910597&view=diff
==============================================================================
--- apr/apr/trunk/shmem/unix/shm.c (original)
+++ apr/apr/trunk/shmem/unix/shm.c Tue Feb 16 17:07:09 2010
@@ -314,26 +314,31 @@
* exist before calling ftok(). */
new_m->shmkey = ftok(filename, 1);
if (new_m->shmkey == (key_t)-1) {
+ apr_file_close(file);
return errno;
}
if ((new_m->shmid = shmget(new_m->shmkey, new_m->realsize,
SHM_R | SHM_W | IPC_CREAT | IPC_EXCL)) < 0) {
+ apr_file_close(file);
return errno;
}
if ((new_m->base = shmat(new_m->shmid, NULL, 0)) == (void *)-1) {
+ apr_file_close(file);
return errno;
}
new_m->usable = new_m->base;
if (shmctl(new_m->shmid, IPC_STAT, &shmbuf) == -1) {
+ apr_file_close(file);
return errno;
}
apr_uid_current(&uid, &gid, pool);
shmbuf.shm_perm.uid = uid;
shmbuf.shm_perm.gid = gid;
if (shmctl(new_m->shmid, IPC_SET, &shmbuf) == -1) {
+ apr_file_close(file);
return errno;
}
@@ -341,6 +346,7 @@
status = apr_file_write(file, (const void *)&reqsize,
&nbytes);
if (status != APR_SUCCESS) {
+ apr_file_close(file);
return status;
}
status = apr_file_close(file);
Re: svn commit: r910597 - /apr/apr/trunk/shmem/unix/shm.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Tuesday, February 16, 2010, Mladen Turk <mt...@apache.org> wrote:
> On 02/16/2010 06:07 PM, jfclere@apache.org wrote:
>
> Log:
> Make sure we don't leak file descriptors.
>
> if (new_m->shmkey == (key_t)-1) {
> + apr_file_close(file);
> return errno;
> }
>
>
> File will be closed when the pool gets destroyed.
> Closing here has little advantages cause the pool
> memory will rise over time so you cannot be sure
> the provided pool won't get dirty if open fails.
Makes sense here
--
Born in Roswell... married an alien...
Re: svn commit: r910597 - /apr/apr/trunk/shmem/unix/shm.c
Posted by Mladen Turk <mt...@apache.org>.
On 02/16/2010 06:07 PM, jfclere@apache.org wrote:
> Log:
> Make sure we don't leak file descriptors.
>
> if (new_m->shmkey == (key_t)-1) {
> + apr_file_close(file);
> return errno;
> }
File will be closed when the pool gets destroyed.
Closing here has little advantages cause the pool
memory will rise over time so you cannot be sure
the provided pool won't get dirty if open fails.
It would be better to create a sub pool for shared
memory which will ensure that both parent pool
doesn't leak memory and that file is always closed.
Repeating apr_file_close across all possible return
failures just make no sense when we have pools.
Regards
--
^TM