You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rb...@locus.apache.org on 2000/06/03 02:31:15 UTC

cvs commit: apache-2.0/src/lib/apr/locks/unix locks.c locks.h

rbb         00/06/02 17:31:15

  Modified:    src/lib/apr/locks/unix locks.c locks.h
  Log:
  FreeBSD 4.0 doesn't like tempnam, so we are using mkstemp now.  I hope
  this works on all platforms, but if not we'll just use a #ifdef later.
  
  Revision  Changes    Path
  1.32      +3 -1      apache-2.0/src/lib/apr/locks/unix/locks.c
  
  Index: locks.c
  ===================================================================
  RCS file: /home/cvs/apache-2.0/src/lib/apr/locks/unix/locks.c,v
  retrieving revision 1.31
  retrieving revision 1.32
  diff -u -r1.31 -r1.32
  --- locks.c	2000/04/30 21:17:56	1.31
  +++ locks.c	2000/06/03 00:31:13	1.32
  @@ -74,7 +74,9 @@
               new->fname = ap_pstrdup(cont, fname);
           }
           else {
  -            new->fname = ap_pstrdup(cont, tempnam(NULL, NULL));
  +            char *filename = "/tmp/aprXXXXXX";
  +            new->interproc = mkstemp(filename);
  +            new->fname = ap_pstrdup(cont, filename); 
               unlink(new->fname);
   	}
       }
  
  
  
  1.21      +3 -0      apache-2.0/src/lib/apr/locks/unix/locks.h
  
  Index: locks.h
  ===================================================================
  RCS file: /home/cvs/apache-2.0/src/lib/apr/locks/unix/locks.h,v
  retrieving revision 1.20
  retrieving revision 1.21
  diff -u -r1.20 -r1.21
  --- locks.h	2000/04/22 06:16:33	1.20
  +++ locks.h	2000/06/03 00:31:14	1.21
  @@ -85,6 +85,9 @@
   #if HAVE_STDIO_H
   #include <stdio.h>
   #endif
  +#if HAVE_STDLIB_H
  +#include <stdlib.h>
  +#endif
   #if HAVE_FCNTL_H
   #include <fcntl.h>
   #endif
  
  
  

Re: cvs commit: apache-2.0/src/lib/apr/locks/unix locks.c locks.h

Posted by Jeff Trawick <tr...@bellsouth.net>.
You need to include <stdlib.h> for that one.  You didn't call it
correctly, according to the Linux man page:

  Since it will be modified, template  must
  not  be  a string constant, but should be 
  declared as a character array.

Let's slow down a bit :)

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/lib/apr/locks/unix locks.c locks.h

Posted by Jeff Trawick <tr...@bellsouth.net>.
> Date: Fri, 2 Jun 2000 19:23:58 -0700 (PDT)
> From: Greg Stein <gs...@lyra.org>
> 
> Looks good, but it doesn't unlink() the opened file.
> 
> Do we still want that feature?

There is an existing call to unlink() a few lines below what you see
in the diff that covers the USE_FCNTL_SERIALIZE path.

The USE_FLOCK_SERIALIZE support doesn't do unlink() until lock cleanup
time.  I will assume for now that it is supposed to work this way.
The file is opened again in the FLOCK version of
ap_unix_child_init_lock(). 

> 
> The #include <stdlib.h> is superfluous. The include of "locks.h" will
> cover that. (Ryan added stdlib in there)

Oops...  I didn't get all of his update.

> 
> Cheers,
> -g

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/lib/apr/locks/unix locks.c locks.h

Posted by Greg Stein <gs...@lyra.org>.
Looks good, but it doesn't unlink() the opened file.

Do we still want that feature?

The #include <stdlib.h> is superfluous. The include of "locks.h" will
cover that. (Ryan added stdlib in there)

Cheers,
-g


On Fri, 2 Jun 2000, Jeff Trawick wrote:
>...
> I think a patch like this would solve it:
> 
> (not tested, but that never stopped anybody from posting or committing
> code :) )
> 
> (seriously, I don't mind testing it, but I imagine that Ryan is
> playing with it now... Ryan, let me know if I should verify and
> commit; Greg will certainly speak up if it is obviously FUBAR)
> 
> Index: src/lib/apr/locks/unix/crossproc.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/lib/apr/locks/unix/crossproc.c,v
> retrieving revision 1.27
> diff -u -r1.27 crossproc.c
> --- crossproc.c	2000/04/14 15:58:30	1.27
> +++ crossproc.c	2000/06/03 02:07:24
> @@ -53,6 +53,7 @@
>   */
>  
>  #include "locks.h"
> +#include <stdlib.h>
>  
>  #if defined (USE_SYSVSEM_SERIALIZE)  
>  
> @@ -269,7 +270,13 @@
>  
>  ap_status_t ap_unix_create_inter_lock(ap_lock_t *new)
>  {
> -    new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
> +    if (new->fname) {
> +        new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
> +    }
> +    else {
> +        new->fname = ap_pstrdup(new->cntxt, "/tmp/aprXXXXXX"); 
> +        new->interproc = mkstemp(new->fname);
> +    }
>  
>      if (new->interproc < 0) {
>          lock_cleanup(new);
> @@ -338,7 +345,13 @@
>  
>  ap_status_t ap_unix_create_inter_lock(ap_lock_t *new)
>  {
> -    new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0600);
> +    if (new->fname) {
> +        new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
> +    }
> +    else {
> +        new->fname = ap_pstrdup(new->cntxt, "/tmp/aprXXXXXX"); 
> +        new->interproc = mkstemp(new->fname);
> +    }
>  
>      if (new->interproc < 0) {
>          lock_cleanup(new);
> Index: src/lib/apr/locks/unix/locks.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/lib/apr/locks/unix/locks.c,v
> retrieving revision 1.32
> diff -u -r1.32 locks.c
> --- locks.c	2000/06/03 00:31:13	1.32
> +++ locks.c	2000/06/03 02:07:24
> @@ -73,12 +73,6 @@
>          if (fname != NULL) {
>              new->fname = ap_pstrdup(cont, fname);
>          }
> -        else {
> -            char *filename = "/tmp/aprXXXXXX";
> -            new->interproc = mkstemp(filename);
> -            new->fname = ap_pstrdup(cont, filename); 
> -            unlink(new->fname);
> -	}
>      }
>  #endif
>  
> 
> 
> 
> -- 
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
>      http://www.geocities.com/SiliconValley/Park/9289/
>           Born in Roswell... married an alien...
> 

-- 
Greg Stein, http://www.lyra.org/



Re: cvs commit: apache-2.0/src/lib/apr/locks/unix locks.c locks.h

Posted by Jeff Trawick <tr...@bellsouth.net>.
> Date: Fri, 2 Jun 2000 17:45:36 -0700 (PDT)
> From: Greg Stein <gs...@lyra.org>
> Two problems:
> 
> 1) mkstemp() modifies its argument. The above code will segfault since
>    "/tmp/aprXXXXXX" is constant string data.
> 
>    I changed the code to look like:
> 
>         else {
>             new->fname = ap_pstrdup(cont, "/tmp/aprXXXXXX"); 
>             new->interproc = mkstemp(new->fname);
>             unlink(new->fname);
> 	}
> 
> 
> 2) mkstemp() opens the file, yet ap_unix_create_inter_lock() will open the
>    file again (and fail, I presume)
> 
>    This part messed me up (not familiar enough with the code), so I didn't
>    check in the above change.
> 
> 
> Cheers,
> -g
> 
> -- 
> Greg Stein, http://www.lyra.org/

I think a patch like this would solve it:

(not tested, but that never stopped anybody from posting or committing
code :) )

(seriously, I don't mind testing it, but I imagine that Ryan is
playing with it now... Ryan, let me know if I should verify and
commit; Greg will certainly speak up if it is obviously FUBAR)

Index: src/lib/apr/locks/unix/crossproc.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/locks/unix/crossproc.c,v
retrieving revision 1.27
diff -u -r1.27 crossproc.c
--- crossproc.c	2000/04/14 15:58:30	1.27
+++ crossproc.c	2000/06/03 02:07:24
@@ -53,6 +53,7 @@
  */
 
 #include "locks.h"
+#include <stdlib.h>
 
 #if defined (USE_SYSVSEM_SERIALIZE)  
 
@@ -269,7 +270,13 @@
 
 ap_status_t ap_unix_create_inter_lock(ap_lock_t *new)
 {
-    new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
+    if (new->fname) {
+        new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
+    }
+    else {
+        new->fname = ap_pstrdup(new->cntxt, "/tmp/aprXXXXXX"); 
+        new->interproc = mkstemp(new->fname);
+    }
 
     if (new->interproc < 0) {
         lock_cleanup(new);
@@ -338,7 +345,13 @@
 
 ap_status_t ap_unix_create_inter_lock(ap_lock_t *new)
 {
-    new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0600);
+    if (new->fname) {
+        new->interproc = open(new->fname, O_CREAT | O_WRONLY | O_EXCL, 0644);
+    }
+    else {
+        new->fname = ap_pstrdup(new->cntxt, "/tmp/aprXXXXXX"); 
+        new->interproc = mkstemp(new->fname);
+    }
 
     if (new->interproc < 0) {
         lock_cleanup(new);
Index: src/lib/apr/locks/unix/locks.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/lib/apr/locks/unix/locks.c,v
retrieving revision 1.32
diff -u -r1.32 locks.c
--- locks.c	2000/06/03 00:31:13	1.32
+++ locks.c	2000/06/03 02:07:24
@@ -73,12 +73,6 @@
         if (fname != NULL) {
             new->fname = ap_pstrdup(cont, fname);
         }
-        else {
-            char *filename = "/tmp/aprXXXXXX";
-            new->interproc = mkstemp(filename);
-            new->fname = ap_pstrdup(cont, filename); 
-            unlink(new->fname);
-	}
     }
 #endif
 



-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/lib/apr/locks/unix locks.c locks.h

Posted by Greg Stein <gs...@lyra.org>.
On 3 Jun 2000 rbb@locus.apache.org wrote:
> rbb         00/06/02 17:31:15
> 
>   Modified:    src/lib/apr/locks/unix locks.c locks.h
>   Log:
>   FreeBSD 4.0 doesn't like tempnam, so we are using mkstemp now.  I hope
>   this works on all platforms, but if not we'll just use a #ifdef later.
>   
>   Revision  Changes    Path
>   1.32      +3 -1      apache-2.0/src/lib/apr/locks/unix/locks.c
>   
>   Index: locks.c
>   ===================================================================
>   RCS file: /home/cvs/apache-2.0/src/lib/apr/locks/unix/locks.c,v
>   retrieving revision 1.31
>   retrieving revision 1.32
>   diff -u -r1.31 -r1.32
>   --- locks.c	2000/04/30 21:17:56	1.31
>   +++ locks.c	2000/06/03 00:31:13	1.32
>   @@ -74,7 +74,9 @@
>                new->fname = ap_pstrdup(cont, fname);
>            }
>            else {
>   -            new->fname = ap_pstrdup(cont, tempnam(NULL, NULL));
>   +            char *filename = "/tmp/aprXXXXXX";
>   +            new->interproc = mkstemp(filename);
>   +            new->fname = ap_pstrdup(cont, filename); 
>                unlink(new->fname);

Two problems:

1) mkstemp() modifies its argument. The above code will segfault since
   "/tmp/aprXXXXXX" is constant string data.

   I changed the code to look like:

        else {
            new->fname = ap_pstrdup(cont, "/tmp/aprXXXXXX"); 
            new->interproc = mkstemp(new->fname);
            unlink(new->fname);
	}


2) mkstemp() opens the file, yet ap_unix_create_inter_lock() will open the
   file again (and fail, I presume)

   This part messed me up (not familiar enough with the code), so I didn't
   check in the above change.


Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/