Re: [PATCH] 64bit compiler issues

2002-07-17 Thread Peter Poeml 

Hi, 

On Tue, Jul 16, 2002 at 06:01:08PM -0700, Greg Stein wrote:
 On Tue, Jul 16, 2002 at 10:31:46AM -0500, William A. Rowe, Jr. wrote:
  At 07:23 PM 7/15/2002, Ryan Bloom wrote:
  
  We could force the size, by using apr_int32_t.  The problem that he is
 
 There is no need to force the size. The value is a simple integer. There is
 no need to make it a long, and that integer will always be castable into a
 void*, and then back (when we pull it out of the hash table).
 
 I see no purpose in the patch's casting to a long. What exactly is that
 solving? We need a response from Peter.

Sorry for keeping you waiting on this.

[...]
 * mod_dav's namespace indexes are integers
 * we stuff those into a hash table
 * we extract them from a hash table
 
 Peter's patch changes the indexes to longs. Why?

The code assumes that a pointer == int == 32 bit.  While in fact int is
32 bit wide on all architectures, a pointer is sized long on 64 bit
architectures...  What the compiler sees is that a pointer is put into an
incompatible type. It warns, and it can't know which actual values will
occur at runtime.  The patch was meant to take care of these possibly
dangerous cases, while ignoring the more obvious harmless ones.

But I'm certainly okay if you say that all these values are indices that
actually never grow larger than int -- it's you who have the deep
understanding of the code, not me. So, if you say that long wasts too
many resources, we'll have to live with the compiler warnings; but at
least we know that the could should work :^}

Anyway, even if the code is safe in all places, one could argue that it
is not written in a clean way, making assumptions about the pointer
size. I would find it desirable if we could get rid of all benign
warnings, so that actual harmful ones do not get lost in the noise. 


The stderr output of make (using gcc 3.1.1 20020708) is this:

protocol.c: In function `ap_note_digest_auth_failure':
protocol.c:1102: warning: long long unsigned int format, apr_time_t arg (arg 4)
http_core.c: In function `chunk_filter':
http_core.c:216: warning: long long unsigned int format, long unsigned int arg (arg 4)
http_protocol.c: In function `ap_byterange_filter':
http_protocol.c:2839: warning: long long unsigned int format, apr_time_t arg (arg 3)
ab.c: In function `output_html_results':
ab.c:1103: warning: long long int format, long int arg (arg 5)
ab.c:1103: warning: long long int format, long int arg (arg 6)
mod_disk_cache.c:431: warning: `remove_url' defined but not used
mod_mem_cache.c: In function `set_max_cache_size':
mod_mem_cache.c:1049: warning: int format, different type arg (arg 3)
mod_mem_cache.c: In function `set_min_cache_object_size':
mod_mem_cache.c:1060: warning: int format, different type arg (arg 3)
mod_mem_cache.c: In function `set_max_cache_object_size':
mod_mem_cache.c:1071: warning: int format, different type arg (arg 3)
mod_mem_cache.c: In function `set_max_object_count':
mod_mem_cache.c:1082: warning: int format, different type arg (arg 3)
cache_pqueue.c: In function `cache_pq_dump':
cache_pqueue.c:259: warning: int format, different type arg (arg 7)
mod_log_config.c: In function `log_request_duration':
mod_log_config.c:548: warning: long long int format, long int arg (arg 3)
mod_log_config.c: In function `log_request_duration_microseconds':
mod_log_config.c:553: warning: long long int format, apr_time_t arg (arg 3)
mod_expires.c: In function `add_expires':
mod_expires.c:502: warning: long long int format, long int arg (arg 3)
mod_headers.c: In function `header_request_duration':
mod_headers.c:185: warning: long long int format, apr_time_t arg (arg 3)
mod_headers.c: In function `header_request_time':
mod_headers.c:189: warning: long long int format, apr_time_t arg (arg 3)
mod_usertrack.c: In function `make_cookie':
mod_usertrack.c:149: warning: long long int format, apr_time_t arg (arg 5)
ssl_scache_shmcb.c: In function `ssl_scache_shmcb_init':
ssl_scache_shmcb.c:391: warning: unsigned int format, different type arg (arg 7)
ssl_scache_shmht.c: In function `ssl_scache_shmht_init':
ssl_scache_shmht.c:177: warning: int format, different type arg (arg 8)
liveprop.c: In function `dav_register_liveprop_namespace':
liveprop.c:83: warning: cast from pointer to integer of different size
liveprop.c:91: warning: cast to pointer from integer of different size
liveprop.c: In function `dav_get_liveprop_ns_index':
liveprop.c:96: warning: cast from pointer to integer of different size
liveprop.c: In function `dav_add_all_liveprop_xmlns':
liveprop.c:115: warning: cast from pointer to integer of different size
mod_cgid.c: In function `close_unix_socket':
mod_cgid.c:938: warning: cast from pointer to integer of different size
mod_cgid.c: In function `connect_to_daemon':
mod_cgid.c:981: warning: cast to pointer from integer of different size
mod_cgid.c: In function `cgid_handler':
mod_cgid.c:1241: warning: cast to pointer from integer of different size
mod_cgid.c:1253: warning: cast to pointer

Re: [PATCH] 64bit compiler issues

2002-07-17 Thread Jeff Trawick 

Peter Poeml [EMAIL PROTECTED] writes:

 The stderr output of make (using gcc 3.1.1 20020708) is this:

a number of these are easily fixable and don't deal with passing an
int in a ptr field...  I'll see what I can do this a.m. and you get to
see how many of my attempts actually helped :)

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

RE: [PATCH] 64bit compiler issues

2002-07-16 Thread Ryan Bloom 


 From: Greg Stein [mailto:[EMAIL PROTECTED]]
 
 On Mon, Jul 15, 2002 at 02:19:06PM +0200, Peter Poeml wrote:
  Hi,
 
  building httpd-2.0.39 on x86_64 (AMD's upcoming 64 bit architecture)
  there are a few compiler warnings, e.g. due to misfitting type
casts.
 
  While some of the warnings can be ignored, I believe that the
attached
  patch fixes the relevant issues.
 
 Nope, sorry. All of those values (namespace IDs) are integer sized.
Making
 them a long is just too big.
 
 What is the specific error that you were seeing? I would guess that
the
 problem can be fixed a bit differently, without changing the type of
those
 indexes.

We could force the size, by using apr_int32_t.  The problem that he is
having, is that pointers on _most_ 64-bit machines (Windows is a notable
exception, there may be others), are 64-bits long.  But we are using
int's, which are 32-bits for the pointers.  We have the added problem
that throughout the code, we pass in integers for void *'s.   :-(

Just specify the exact size you want, and this problem should go away.

Ryan

RE: [PATCH] 64bit compiler issues

2002-07-16 Thread Ryan Bloom 

 From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]]
 
 At 07:23 PM 7/15/2002, Ryan Bloom wrote:
 
 We could force the size, by using apr_int32_t.  The problem that he
is
 having, is that pointers on _most_ 64-bit machines (Windows is a
notable
 exception, there may be others), are 64-bits long.  But we are using
 int's, which are 32-bits for the pointers.  We have the added problem
 that throughout the code, we pass in integers for void *'s.   :-(
 
 Transposed that statement ;-/
 
 Pointers on Win64 are 64 bits long, just like you expected.
 
 Both int and long remain 32 bits long, unlike what you might expect
 (and certainly different from Unix.)

Ah.   Okay, thanks for correcting me.

 When we mean a void*, we need to spell out void*.  If we need to
 pass it through an integer, prove it, and we will consider an
 apr_intptr_t type, that could be nothing more complicated than
 a union of the appropriate int type and void* for a given platform.

Unfortunately, it isn't something we need to do, just something we have
done throughout the code for convenience.:-(

Ryan

Re: [PATCH] 64bit compiler issues

2002-07-16 Thread Greg Stein 

On Tue, Jul 16, 2002 at 10:31:46AM -0500, William A. Rowe, Jr. wrote:
 At 07:23 PM 7/15/2002, Ryan Bloom wrote:
 
 We could force the size, by using apr_int32_t.  The problem that he is

There is no need to force the size. The value is a simple integer. There is
no need to make it a long, and that integer will always be castable into a
void*, and then back (when we pull it out of the hash table).

I see no purpose in the patch's casting to a long. What exactly is that
solving? We need a response from Peter.

 having, is that pointers on _most_ 64-bit machines (Windows is a notable
 exception, there may be others), are 64-bits long.  But we are using
 int's, which are 32-bits for the pointers.  We have the added problem
 that throughout the code, we pass in integers for void *'s.   :-(

No. You've got that backwards. We're putting integers into points, and then
going back to integers. We are *not* attemping to stuff a pointer into an
integer.

...
 When we mean a void*, we need to spell out void*.  If we need to
 pass it through an integer, prove it, and we will consider an

We aren't. Ryan set you off on a red herring.


* mod_dav's namespace indexes are integers
* we stuff those into a hash table
* we extract them from a hash table

Peter's patch changes the indexes to longs. Why?

Cheers,
-g

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

Re: [PATCH] 64bit compiler issues

2002-07-15 Thread Peter Poeml 

On Mon, Jul 15, 2002 at 02:19:06PM +0200, Peter Poeml wrote:
 Hi, 
 
 building httpd-2.0.39 on x86_64 (AMD's upcoming 64 bit architecture)
 there are a few compiler warnings, e.g. due to misfitting type casts. 

I just noticed that the patch I sent you doesn't apply against the CVS
head branch anymore. Sorry about that.

Please find a revised patch attached!

Thanks,
Peter

-- 
VFS: Busy inodes after unmount. Self-destruct in 5 seconds.  Have a nice day...


Index: modules/dav/fs/dbm.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/dav/fs/dbm.c,v
retrieving revision 1.25
diff -u -r1.25 dbm.c
--- modules/dav/fs/dbm.c10 Jul 2002 06:01:10 -  1.25
+++ modules/dav/fs/dbm.c15 Jul 2002 12:30:57 -
 -344,7 +344,7 
l_ns = 0;
 }
 else {
-int ns_id = (int)apr_hash_get(db-uri_index, name-ns,
+long ns_id = (long)apr_hash_get(db-uri_index, name-ns,
   APR_HASH_KEY_STRING);
 
 
 -353,7 +353,7 
 return key; /* zeroed */
 }
 
-l_ns = sprintf(nsbuf, %d, ns_id - 1);
+l_ns = sprintf(nsbuf, %ld, ns_id - 1);
 }
 
 /* assemble: #:name */
 -608,7 +608,7 
 
 const char *uri = *puri;
 apr_size_t uri_len = strlen(uri);
-int ns_id = (int)apr_hash_get(db-uri_index, uri, uri_len);
+long ns_id = (long)apr_hash_get(db-uri_index, uri, uri_len);
 
 if (ns_id == 0) {
 dav_check_bufsize(db-pool, db-ns_table, uri_len + 1);
Index: modules/dav/fs/repos.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/dav/fs/repos.c,v
retrieving revision 1.70
diff -u -r1.70 repos.c
--- modules/dav/fs/repos.c  23 Jun 2002 06:46:52 -  1.70
+++ modules/dav/fs/repos.c  15 Jul 2002 12:30:58 -
 -1829,7 +1829,7 
 const char *s;
 apr_pool_t *p = resource-info-pool;
 const dav_liveprop_spec *info;
-int global_ns;
+long global_ns;
 
 /* an HTTP-date can be 29 chars plus a null term */
 /* a 64-bit size can be 20 chars plus a null term */
 -1910,11 +1910,11 
 /* DBG3(FS: inserting lp%d:%s  (local %d), ns, scan-name, scan-ns); */
 
 if (what == DAV_PROP_INSERT_VALUE) {
-   s = apr_psprintf(p, lp%d:%s%s/lp%d:%s DEBUG_CR,
+   s = apr_psprintf(p, lp%ld:%s%s/lp%ld:%s DEBUG_CR,
  global_ns, info-name, value, global_ns, info-name);
 }
 else if (what == DAV_PROP_INSERT_NAME) {
-   s = apr_psprintf(p, lp%d:%s/ DEBUG_CR, global_ns, info-name);
+   s = apr_psprintf(p, lp%ld:%s/ DEBUG_CR, global_ns, info-name);
 }
 else {
 /* assert: what == DAV_PROP_INSERT_SUPPORTED */
Index: modules/dav/main/liveprop.c
===
RCS file: /home/cvspublic/httpd-2.0/modules/dav/main/liveprop.c,v
retrieving revision 1.13
diff -u -r1.13 liveprop.c
--- modules/dav/main/liveprop.c 23 Jun 2002 06:42:13 -  1.13
+++ modules/dav/main/liveprop.c 15 Jul 2002 12:30:58 -
 -73,14 +73,14 
 
 static void dav_register_liveprop_namespace(apr_pool_t *p, const char *uri)
 {
-int value;
+long value;
 
 if (dav_liveprop_uris == NULL) {
 dav_liveprop_uris = apr_hash_make(p);
 apr_pool_cleanup_register(p, NULL, dav_cleanup_liveprops, 
apr_pool_cleanup_null);
 }
 
-value = (int)apr_hash_get(dav_liveprop_uris, uri, APR_HASH_KEY_STRING);
+value = (long)apr_hash_get(dav_liveprop_uris, uri, APR_HASH_KEY_STRING);
 if (value != 0) {
 /* already registered */
 return;
 -91,9 +91,9 
  (void *)++dav_liveprop_count);
 }
 
-DAV_DECLARE(int) dav_get_liveprop_ns_index(const char *uri)
+DAV_DECLARE(long) dav_get_liveprop_ns_index(const char *uri)
 {
-return (int)apr_hash_get(dav_liveprop_uris, uri, APR_HASH_KEY_STRING);
+return (long)apr_hash_get(dav_liveprop_uris, uri, APR_HASH_KEY_STRING);
 }
 
 int dav_get_liveprop_ns_count(void)
 -112,7 +112,7 
 
 apr_hash_this(idx, key, NULL, val);
 
-s = apr_psprintf(p,  xmlns:lp%d=\%s\, (int)val, (const char *)key);
+s = apr_psprintf(p,  xmlns:lp%ld=\%s\, (long)val, (const char *)key);
 apr_text_append(p, phdr, s);
 }
 }
 -145,7 +145,7 
 return 0;
 }
 
-DAV_DECLARE(int) dav_get_liveprop_info(int propid,
+DAV_DECLARE(long) dav_get_liveprop_info(int propid,
const dav_liveprop_group *group,
const dav_liveprop_spec **info)
 {
Index: modules/dav/main/mod_dav.h
===
RCS file: /home/cvspublic/httpd-2.0/modules/dav/main/mod_dav.h,v
retrieving revision 1.63
diff -u -r1.63 mod_dav.h
--- modules/dav/main/mod_dav.h  23 Jun 2002 06:42:13 -  1.63
+++ modules/dav/main/mod_dav.h  15 Jul 2002 12:30:58 -
 -899,7 +899,7

Re: [PATCH] 64bit compiler issues

2002-07-15 Thread Greg Stein 

On Mon, Jul 15, 2002 at 02:19:06PM +0200, Peter Poeml wrote:
 Hi, 
 
 building httpd-2.0.39 on x86_64 (AMD's upcoming 64 bit architecture)
 there are a few compiler warnings, e.g. due to misfitting type casts. 
 
 While some of the warnings can be ignored, I believe that the attached
 patch fixes the relevant issues.

Nope, sorry. All of those values (namespace IDs) are integer sized. Making
them a long is just too big.

What is the specific error that you were seeing? I would guess that the
problem can be fixed a bit differently, without changing the type of those
indexes.

Cheers,
-g

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