You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by ak...@apache.org on 2004/10/07 21:28:55 UTC
cvs commit: apr/memory/unix apr_pools.c
ake 2004/10/07 12:28:55
Modified: include apr.hnw apr.hw
include/arch apr_private_common.h
include/arch/netware apr_private.h
include/arch/win32 apr_private.h
memory/unix apr_pools.c
Log:
WIN64: update pools code for 64 bit compiles
Revision Changes Path
1.50 +0 -1 apr/include/apr.hnw
Index: apr.hnw
===================================================================
RCS file: /home/cvs/apr/include/apr.hnw,v
retrieving revision 1.49
retrieving revision 1.50
diff -u -r1.49 -r1.50
--- apr.hnw 1 Oct 2004 19:24:03 -0000 1.49
+++ apr.hnw 7 Oct 2004 19:28:54 -0000 1.50
@@ -330,7 +330,6 @@
#define APR_UINT64_T_HEX_FMT "llx"
#define APR_TIME_T_FMT APR_INT64_T_FMT
-#define APR_DWORD_MAX 0xFFFFFFFFUL /* 2^32*/
/** @} */
#ifdef __cplusplus
1.128 +0 -2 apr/include/apr.hw
Index: apr.hw
===================================================================
RCS file: /home/cvs/apr/include/apr.hw,v
retrieving revision 1.127
retrieving revision 1.128
diff -u -r1.127 -r1.128
--- apr.hw 1 Oct 2004 19:24:03 -0000 1.127
+++ apr.hw 7 Oct 2004 19:28:54 -0000 1.128
@@ -342,8 +342,6 @@
#define APR_SIZEOF_VOIDP 4
#endif
-#define APR_DWORD_MAX 0xFFFFFFFFUL
-
/* XXX These simply don't belong here, perhaps in apr_portable.h
* based on some APR_HAVE_PID/GID/UID?
*/
1.3 +5 -0 apr/include/arch/apr_private_common.h
Index: apr_private_common.h
===================================================================
RCS file: /home/cvs/apr/include/arch/apr_private_common.h,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- apr_private_common.h 13 Feb 2004 09:38:29 -0000 1.2
+++ apr_private_common.h 7 Oct 2004 19:28:54 -0000 1.3
@@ -33,4 +33,9 @@
char separator,
apr_pool_t *p);
+/* temporary defines to handle 64bit compile mismatches */
+#define APR_INT_TRUNC_CAST int
+#define APR_UINT32_TRUNC_CAST apr_uint32_t
+#define APR_UINT32_MAX 0xFFFFFFFFUL
+
#endif /*APR_PRIVATE_COMMON_H*/
1.27 +3 -0 apr/include/arch/netware/apr_private.h
Index: apr_private.h
===================================================================
RCS file: /home/cvs/apr/include/arch/netware/apr_private.h,v
retrieving revision 1.26
retrieving revision 1.27
diff -u -r1.26 -r1.27
--- apr_private.h 6 Oct 2004 22:20:22 -0000 1.26
+++ apr_private.h 7 Oct 2004 19:28:54 -0000 1.27
@@ -174,6 +174,9 @@
#define APR_OFF_T_STRFN strtol
#endif
+/* used to check DWORD overflow for 64bit compiles */
+#define APR_DWORD_MAX 0xFFFFFFFFUL
+
/*
* Include common private declarations.
*/
1.39 +3 -0 apr/include/arch/win32/apr_private.h
Index: apr_private.h
===================================================================
RCS file: /home/cvs/apr/include/arch/win32/apr_private.h,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -r1.38 -r1.39
--- apr_private.h 4 Jun 2004 23:26:02 -0000 1.38
+++ apr_private.h 7 Oct 2004 19:28:54 -0000 1.39
@@ -158,6 +158,9 @@
#define APR_OFF_T_STRFN strtoi
#endif
+/* used to check for DWORD overflow in 64bit compiles */
+#define APR_DWORD_MAX 0xFFFFFFFFUL
+
/*
* Include common private declarations.
*/
1.207 +15 -9 apr/memory/unix/apr_pools.c
Index: apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.206
retrieving revision 1.207
diff -u -r1.206 -r1.207
--- apr_pools.c 17 Jun 2004 14:13:58 -0000 1.206
+++ apr_pools.c 7 Oct 2004 19:28:55 -0000 1.207
@@ -139,9 +139,10 @@
}
APR_DECLARE(void) apr_allocator_max_free_set(apr_allocator_t *allocator,
- apr_size_t size)
+ apr_size_t in_size)
{
apr_uint32_t max_free_index;
+ apr_uint32_t size = (APR_UINT32_TRUNC_CAST)in_size;
#if APR_HAS_THREADS
apr_thread_mutex_t *mutex;
@@ -168,7 +169,8 @@
apr_memnode_t *allocator_alloc(apr_allocator_t *allocator, apr_size_t size)
{
apr_memnode_t *node, **ref;
- apr_uint32_t i, index, max_index;
+ apr_uint32_t max_index;
+ apr_size_t i, index;
/* Round up the block size to the next boundary, but always
* allocate at least a certain size (MIN_ALLOC).
@@ -181,6 +183,10 @@
* dividing its size by the boundary size
*/
index = (size >> BOUNDARY_INDEX) - 1;
+
+ if (index > APR_UINT32_MAX) {
+ return NULL;
+ }
/* First see if there are any nodes in the area we know
* our node will fit into.
@@ -293,7 +299,7 @@
return NULL;
node->next = NULL;
- node->index = index;
+ node->index = (APR_UINT32_TRUNC_CAST)index;
node->first_avail = (char *)node + APR_MEMNODE_T_SIZE;
node->endp = (char *)node + size;
@@ -582,7 +588,7 @@
{
apr_memnode_t *active, *node;
void *mem;
- apr_uint32_t free_index;
+ apr_size_t free_index;
size = APR_ALIGN_DEFAULT(size);
active = pool->active;
@@ -620,7 +626,7 @@
free_index = (APR_ALIGN(active->endp - active->first_avail + 1,
BOUNDARY_SIZE) - BOUNDARY_SIZE) >> BOUNDARY_INDEX;
- active->free_index = free_index;
+ active->free_index = (APR_UINT32_TRUNC_CAST)free_index;
node = active->next;
if (free_index >= node->free_index)
return mem;
@@ -877,7 +883,7 @@
apr_size_t cur_len, size;
char *strp;
apr_pool_t *pool;
- apr_uint32_t free_index;
+ apr_size_t free_index;
pool = ps->pool;
active = ps->node;
@@ -907,7 +913,7 @@
free_index = (APR_ALIGN(active->endp - active->first_avail + 1,
BOUNDARY_SIZE) - BOUNDARY_SIZE) >> BOUNDARY_INDEX;
- active->free_index = free_index;
+ active->free_index = (APR_UINT32_TRUNC_CAST)free_index;
node = active->next;
if (free_index < node->free_index) {
do {
@@ -948,7 +954,7 @@
char *strp;
apr_size_t size;
apr_memnode_t *active, *node;
- apr_uint32_t free_index;
+ apr_size_t free_index;
ps.node = active = pool->active;
ps.pool = pool;
@@ -1008,7 +1014,7 @@
free_index = (APR_ALIGN(active->endp - active->first_avail + 1,
BOUNDARY_SIZE) - BOUNDARY_SIZE) >> BOUNDARY_INDEX;
- active->free_index = free_index;
+ active->free_index = (APR_UINT32_TRUNC_CAST)free_index;
node = active->next;
if (free_index >= node->free_index)
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Friday, October 8, 2004 4:31 PM +0100 Joe Orton <jo...@redhat.com> wrote:
> I think comments would be much clearer because they'd remove the "huh?"
> factor when you read the code for the first time without knowing what
> the macro means. But I don't care enough to go and revert the changes
> and probably nobody else is paying attention so I guess let's give up
> and go do something useful.
FWIW, I agree with Joe. The casts are evil in this particular case and
serious consideration should be made to backing it out. We should just track
it in STATUS instead... -- justin
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Oct 08, 2004 at 11:22:00AM -0400, Jeff Trawick wrote:
> If you agree with the assumption that we can't throw away the reason
> for the cast but need something in the source file to help us
> remember, then it becomes a question of the clearest way to preserve
> the information.
>
> To me, using a special cast like that is as clear as a comment, more
> concise than a comment, and more likely to be consistently applied and
> easily found when looking for these problems than a comment.
I think comments would be much clearer because they'd remove the "huh?"
factor when you read the code for the first time without knowing what
the macro means. But I don't care enough to go and revert the changes
and probably nobody else is paying attention so I guess let's give up
and go do something useful.
joe
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, 8 Oct 2004 15:08:38 +0100, Joe Orton <jo...@redhat.com> wrote:
>
>
> On Fri, Oct 08, 2004 at 09:58:07AM -0400, Allan Edwards wrote:
> > Joe Orton wrote:
> > >>The fact that these casts are unnecessary can be tracked using comments,
> > >>a list of issues in STATUS, or, not wishing to rock the boat, bugzilla.
> > >>Using a macro just obfuscates the code.
> > >
> > >
> > >s/unnecessary/undesirable/
> >
> > I hardly think it is more obfuscating that any
> > macro might be considered to be. The only reasonable
> > alternative in my opinion would be to add a comment
> > line for each cast with some common grep-able
> > string. But to me that is uglier.
>
> A macro is useful if it abstracts something away. This macro does not
> abstract anything away. It expands to apr_uint32_t now, and it can
> *never* expand to anything other than apr_uint32_t. That is obfuscation
> in my book.
If you agree with the assumption that we can't throw away the reason
for the cast but need something in the source file to help us
remember, then it becomes a question of the clearest way to preserve
the information.
To me, using a special cast like that is as clear as a comment, more
concise than a comment, and more likely to be consistently applied and
easily found when looking for these problems than a comment.
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Oct 08, 2004 at 09:58:07AM -0400, Allan Edwards wrote:
> Joe Orton wrote:
> >>The fact that these casts are unnecessary can be tracked using comments,
> >>a list of issues in STATUS, or, not wishing to rock the boat, bugzilla.
> >>Using a macro just obfuscates the code.
> >
> >
> >s/unnecessary/undesirable/
>
> I hardly think it is more obfuscating that any
> macro might be considered to be. The only reasonable
> alternative in my opinion would be to add a comment
> line for each cast with some common grep-able
> string. But to me that is uglier.
A macro is useful if it abstracts something away. This macro does not
abstract anything away. It expands to apr_uint32_t now, and it can
*never* expand to anything other than apr_uint32_t. That is obfuscation
in my book.
joe
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Allan Edwards <ak...@us.ibm.com>.
Joe Orton wrote:
>>The fact that these casts are unnecessary can be tracked using comments,
>>a list of issues in STATUS, or, not wishing to rock the boat, bugzilla.
>>Using a macro just obfuscates the code.
>
>
> s/unnecessary/undesirable/
I hardly think it is more obfuscating that any
macro might be considered to be. The only reasonable
alternative in my opinion would be to add a comment
line for each cast with some common grep-able
string. But to me that is uglier.
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Oct 07, 2004 at 10:04:22PM +0100, Joe Orton wrote:
> On Thu, Oct 07, 2004 at 04:54:13PM -0400, Allan Edwards wrote:
> > Joe Orton wrote:
> > >Why is this a macro? It's not like apr_uint32_t is a name which is going
> > >to change any time soon?
> >
> > It's a macro because we don't want to lose sight of the
> > fact that we did something there that we utimately want to
> > back out (i.e. in APR 2.0 when we can change the API). If
> > we used apr_uint32_t it would be easy to lose track of
> > these places - make sense?
>
> The fact that these casts are unnecessary can be tracked using comments,
> a list of issues in STATUS, or, not wishing to rock the boat, bugzilla.
> Using a macro just obfuscates the code.
s/unnecessary/undesirable/
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Oct 07, 2004 at 04:54:13PM -0400, Allan Edwards wrote:
> Joe Orton wrote:
> >Why is this a macro? It's not like apr_uint32_t is a name which is going
> >to change any time soon?
>
> It's a macro because we don't want to lose sight of the
> fact that we did something there that we utimately want to
> back out (i.e. in APR 2.0 when we can change the API). If
> we used apr_uint32_t it would be easy to lose track of
> these places - make sense?
The fact that these casts are unnecessary can be tracked using comments,
a list of issues in STATUS, or, not wishing to rock the boat, bugzilla.
Using a macro just obfuscates the code.
joe
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Allan Edwards <ak...@us.ibm.com>.
Joe Orton wrote:
> Why is this a macro? It's not like apr_uint32_t is a name which is going
> to change any time soon?
It's a macro because we don't want to lose sight of the
fact that we did something there that we utimately want to
back out (i.e. in APR 2.0 when we can change the API). If
we used apr_uint32_t it would be easy to lose track of
these places - make sense?
Allan
Re: cvs commit: apr/memory/unix apr_pools.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Oct 07, 2004 at 07:28:55PM -0000, ake@apache.org wrote:
> +/* temporary defines to handle 64bit compile mismatches */
> +#define APR_INT_TRUNC_CAST int
> +#define APR_UINT32_TRUNC_CAST apr_uint32_t
> +#define APR_UINT32_MAX 0xFFFFFFFFUL
> +
...
> }
>
> APR_DECLARE(void) apr_allocator_max_free_set(apr_allocator_t *allocator,
> - apr_size_t size)
> + apr_size_t in_size)
> {
> apr_uint32_t max_free_index;
> + apr_uint32_t size = (APR_UINT32_TRUNC_CAST)in_size;
Why is this a macro? It's not like apr_uint32_t is a name which is going
to change any time soon?
joe