Re: [PATCH] 64bit compiler issues
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
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: warni
Re: [PATCH] 64bit compiler issues
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
> 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
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.) http://www.intel.com/design/itanium/getsoftready/sld011.htm http://www.devx.com/Itanium/art_Porting2.asp The biggest difference is the long value. Since Windows is a P64 OS, rather than the Linux PL64 (both pointers and longs of 64 bits) long becomes a rather dangerous type if you are assuming that it's interchangeable with a pointer. 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. Bill
RE: [PATCH] 64bit compiler issues
> 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
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/
Re: [PATCH] 64bit compiler issues
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, "%s" DEBUG_CR, + s = apr_psprintf(p, "%s" DEBUG_CR, global_ns, info->name, value, global_ns, info->name); } else if (what == DAV_PROP_INSERT_NAME) { - s = apr_psprintf(p, "" DEBUG_CR, global_ns, info->name); + s = apr_psprintf(p, "" 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 -0