httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@attglobal.net>
Subject Re: [PATCH] fix alignment on shared memory
Date Fri, 01 Mar 2002 18:54:23 GMT
Cliff Woolley <jwoolley@virginia.edu> writes:

> On 1 Mar 2002, Jeff Trawick wrote:
> 
> > Jeff calling Cliff and Aaron: apr_shm_baseaddr_get() returns
> > addresses which aren't 64-bit aligned.  That is broken.  End of story.
> 
> Yeah, your last message cleared a lot up.  Thanks.  But I believe we're
> all right.  Both things are broken.  :)

Hell, I already agreed that the calc-scoreboard-size stuff was
theoretically bad (but not bad with the current structure layouts
since we don't currently lose 64-bit alignment) :)

Here is my latest patch.  Somebody that understands shm.c needs to
check it over and see if a seek() is needed before a mmap().  Also, as
you suggest the align macro needs to be in an APR header file.

Index: srclib/apr/shmem/unix/shm.c
===================================================================
RCS file: /home/cvs/apr/shmem/unix/shm.c,v
retrieving revision 1.14
diff -u -r1.14 shm.c
--- srclib/apr/shmem/unix/shm.c	5 Feb 2002 04:32:52 -0000	1.14
+++ srclib/apr/shmem/unix/shm.c	1 Mar 2002 18:55:59 -0000
@@ -59,6 +59,12 @@
 #include "apr_user.h"
 #include "apr_strings.h"
 
+/* APR_ALIGN() is only to be used to align on a power of 2 boundary */
+#define APR_ALIGN(size, boundary) \
+    (((size) + ((boundary) - 1)) & ~((boundary) - 1))
+
+#define APR_ALIGN_DEFAULT(size) APR_ALIGN(size, 8)
+
 static apr_status_t shm_cleanup_owner(void *m_)
 {
     apr_shm_t *m = (apr_shm_t *)m_;
@@ -161,7 +167,8 @@
         }
         new_m->pool = pool;
         new_m->reqsize = reqsize;
-        new_m->realsize = reqsize + sizeof(apr_size_t); /* room for metadata */
+        new_m->realsize = reqsize + 
+            APR_ALIGN_DEFAULT(sizeof(apr_size_t)); /* room for metadata */
         new_m->filename = NULL;
     
 #if APR_USE_SHMEM_MMAP_ZERO
@@ -189,7 +196,7 @@
         /* store the real size in the metadata */
         *(apr_size_t*)(new_m->base) = new_m->realsize;
         /* metadata isn't usable */
-        new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+        new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
 
         apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
                                   apr_pool_cleanup_null);
@@ -206,7 +213,7 @@
         /* store the real size in the metadata */
         *(apr_size_t*)(new_m->base) = new_m->realsize;
         /* metadata isn't usable */
-        new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+        new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
 
         apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
                                   apr_pool_cleanup_null);
@@ -274,7 +281,8 @@
         new_m->filename = apr_pstrdup(pool, filename);
 
 #if APR_USE_SHMEM_MMAP_TMP || APR_USE_SHMEM_MMAP_SHM
-        new_m->realsize = reqsize + sizeof(apr_size_t); /* room for metadata */
+        new_m->realsize = reqsize + 
+            APR_ALIGN_DEFAULT(sizeof(apr_size_t)); /* room for metadata */
         /* FIXME: Ignore error for now. *
          * status = apr_file_remove(file, pool);*/
         status = APR_SUCCESS;
@@ -345,7 +353,7 @@
         /* store the real size in the metadata */
         *(apr_size_t*)(new_m->base) = new_m->realsize;
         /* metadata isn't usable */
-        new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+        new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
 
         apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_owner,
                                   apr_pool_cleanup_null);
@@ -482,6 +490,9 @@
             return status;
         }
 
+         XXX use APR_ALIGN_DEFAULT() somewhere here?
+         XXX do we need to seek() prior to the mmap()?
+
         nbytes = sizeof(new_m->realsize);
         status = apr_file_read(file, (void *)&(new_m->realsize),
                                &nbytes);
@@ -508,7 +519,7 @@
         }
 
         /* metadata isn't part of the usable segment */
-        new_m->usable = (char *)new_m->base + sizeof(apr_size_t);
+        new_m->usable = (char *)new_m->base + APR_ALIGN_DEFAULT(sizeof(apr_size_t));
 
         apr_pool_cleanup_register(new_m->pool, new_m, shm_cleanup_attach,
                                   apr_pool_cleanup_null);

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

Mime
View raw message