[Freeciv-Dev] (PR#21625) PATCH - improved hash tables
http://bugs.freeciv.org/Ticket/Display.html?id=21625 > > [vasc - Thu Oct 05 22:44:29 2006]: > > Freeciv uses some C99 features. C99 has integer types which are guaranteed > to fit pointer values: > intptr_t and uintptr_t from inttypes.h. I did not know that. The question is can I assume that these C99 features will exist everywhere freeciv is expected to compile? > > This is the ugliest bit I found: > --- > +#undef get16bits > +#if (defined(__GNUC__) && defined(__i386__)) || defined(__WATCOMC__) \ > + || defined(_MSC_VER) || defined (__BORLANDC__) || defined (__TURBOC__) > +#define get16bits(d) (*((const uint16_t *) (d))) > +#endif > + > +#if !defined (get16bits) > +#define get16bits(d) const uint8_t *)(d))[1] << UINT32_C(8))\ > + +((const uint8_t *)(d))[0]) > +#endif > --- > > Basically the first case caters for a little-endian X86 architecture. I am > somewhat unsure what this > macro is supposed to do, but my guess is it is meant to snarf 16-bits in > little endian order. > > I think the second case would work on a big-endian architecture. This sort > of #ifdef'ing is against > autoconf style. The check should IMO be for endianness not any specific > compiler. Alternatively, > forget the little tin god and just use the, more generic, second macro > definition. I agree completely. The only reason this code snippet survived as long as it did is because it was in the original SuperFastHash (which tried to be as self contained as possible and so didn't use autoconf style macros) and did not generate any compile warnings or errors. I'm sure we can spare the few extra cycles to use only the more generic version. :) freeciv_S2_1_r12321-004-intptr_t_for_ptr_conversion_macros.diff Description: Binary data freeciv_S2_1_r12321-004-super_fast_hash_sanitization.diff Description: Binary data ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash tables
http://bugs.freeciv.org/Ticket/Display.html?id=21625 > Freeciv uses some C99 features. C99 has integer types which are guaranteed to fit pointer values: intptr_t and uintptr_t from inttypes.h. Although from a first glace at the code this does not seem to be a problem. This is the ugliest bit I found: --- +#undef get16bits +#if (defined(__GNUC__) && defined(__i386__)) || defined(__WATCOMC__) \ + || defined(_MSC_VER) || defined (__BORLANDC__) || defined (__TURBOC__) +#define get16bits(d) (*((const uint16_t *) (d))) +#endif + +#if !defined (get16bits) +#define get16bits(d) const uint8_t *)(d))[1] << UINT32_C(8))\ + +((const uint8_t *)(d))[0]) +#endif --- Basically the first case caters for a little-endian X86 architecture. I am somewhat unsure what this macro is supposed to do, but my guess is it is meant to snarf 16-bits in little endian order. I think the second case would work on a big-endian architecture. This sort of #ifdef'ing is against autoconf style. The check should IMO be for endianness not any specific compiler. Alternatively, forget the little tin god and just use the, more generic, second macro definition. On 10/5/06, book <[EMAIL PROTECTED]> wrote: > > > http://bugs.freeciv.org/Ticket/Display.html?id=21625 > > > > > - Original Message Follows - > From: "Per I. Mathisen" <[EMAIL PROTECTED]> > To: [EMAIL PROTECTED] > Subject: Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash > tables > Date: Wed, 4 Oct 2006 12:01:18 -0700 > > > Are those pointer conversion macros 64bit safe? For all > > types of common 64bit systems? > > No idea. As I don't have a 64bit system to test, never > programmed on one, and > only made the conversion macros by monkeying glib :). > > Actually a better way to construct the macros would be to > have sizeof check for > pointers in configure.ac and then use AC_DEFINE (or > whatever) to substitute in > the correct cast expression (this is the way glib actually > does it, I got lazy). > > > > > ___ > Freeciv-dev mailing list > Freeciv-dev@gna.org > https://mail.gna.org/listinfo/freeciv-dev > -- Vasco Alexandre da Silva Costa Freeciv uses some C99 features. C99 has integer types which are guaranteed to fit pointer values:intptr_t and uintptr_t from inttypes.h. Although from a first glace at the code this does not seem to be a problem. This is the ugliest bit I found:---+#undef get16bits+#if (defined(__GNUC__) && defined(__i386__)) || defined(__WATCOMC__) \+ || defined(_MSC_VER) || defined (__BORLANDC__) || defined (__TURBOC__) +#define get16bits(d) (*((const uint16_t *) (d)))+#endif++#if !defined (get16bits)+#define get16bits(d) const uint8_t *)(d))[1] << UINT32_C(8))\+ +((const uint8_t *)(d))[0]) +#endif---Basically the first case caters for a little-endian X86 architecture. I am somewhat unsure what thismacro is supposed to do, but my guess is it is meant to snarf 16-bits in little endian order. I think the second case would work on a big-endian architecture. This sort of #ifdef'ing is againstautoconf style. The check should IMO be for endianness not any specific compiler. Alternatively,forget the little tin god and just use the, more generic, second macro definition. On 10/5/06, book <[EMAIL PROTECTED]> wrote: http://bugs.freeciv.org/Ticket/Display.html?id=21625 >- Original Message Follows -From: "Per I. Mathisen" < [EMAIL PROTECTED]>To: [EMAIL PROTECTED]Subject: Re: [Freeciv-Dev] (PR#21625) PATCH - improved hashtablesDate: Wed, 4 Oct 2006 12:01:18 -0700 > Are those pointer conversion macros 64bit safe? For all> types of common 64bit systems?No idea. As I don't have a 64bit system to test, neverprogrammed on one, andonly made the conversion macros by monkeying glib :). Actually a better way to construct the macros would be tohave sizeof check forpointers in configure.ac and then use AC_DEFINE (orwhatever) to substitute inthe correct cast _expression_ (this is the way glib actually does it, I got lazy).___Freeciv-dev mailing listFreeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev-- Vasco Alexandre da Silva Costa ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash tables
http://bugs.freeciv.org/Ticket/Display.html?id=21625 > - Original Message Follows - From: "Per I. Mathisen" <[EMAIL PROTECTED]> To: [EMAIL PROTECTED] Subject: Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash tables Date: Wed, 4 Oct 2006 12:01:18 -0700 >Are those pointer conversion macros 64bit safe? For all >types of common 64bit systems? No idea. As I don't have a 64bit system to test, never programmed on one, and only made the conversion macros by monkeying glib :). Actually a better way to construct the macros would be to have sizeof check for pointers in configure.ac and then use AC_DEFINE (or whatever) to substitute in the correct cast expression (this is the way glib actually does it, I got lazy). _ Express yourself instantly with MSN Messenger! Download today it's FREE! http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash tables
http://bugs.freeciv.org/Ticket/Display.html?id=21625 > - Original Message Follows - From: "Per I. Mathisen" <[EMAIL PROTECTED]> To: [EMAIL PROTECTED] Subject: Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash tables Date: Wed, 4 Oct 2006 12:01:18 -0700 > Are those pointer conversion macros 64bit safe? For all > types of common 64bit systems? No idea. As I don't have a 64bit system to test, never programmed on one, and only made the conversion macros by monkeying glib :). Actually a better way to construct the macros would be to have sizeof check for pointers in configure.ac and then use AC_DEFINE (or whatever) to substitute in the correct cast expression (this is the way glib actually does it, I got lazy). ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#21625) PATCH - improved hash tables
http://bugs.freeciv.org/Ticket/Display.html?id=21625 > Neat. A fast hash iterator can be quite useful. On Wed, 4 Oct 2006, book wrote: > This is a prerequisite patch for my ADNS patches. In order to be used > effectively it requires my safe pointer converions macro patch (21622). Are those pointer conversion macros 64bit safe? For all types of common 64bit systems? - Per ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#21625) PATCH - improved hash tables
http://bugs.freeciv.org/Ticket/Display.html?id=21625 >
This is a prerequisite patch for my ADNS patches. In order to be used
effectively it requires my safe pointer converions macro patch (21622).
Changes:
- Integer hash functions now assume value rather than pointer, the old
hash_f???_int, were renamed to hash_f???_ptr_int for compatibility
(only idex code uses these).
- Added fast general purpose hash function by Paul Hsieh (SuperFastHash)
now used by hash_fval_{int,uint16_t,uint32_t,string2}.
- Added case insensitive string fcmp function (hash_fcmp_string_ci).
- hash_dump function for debugging (commented out at the moment)
- hash_resize_table now emits an error if there is a collision during
resize rather than aborting (though usually benign, this indicates
a programming error, e.g. dangling pointers in hash table key/values).
- hash_tables are no longer automatically shrunk when entries are
removed, deleted; hence resize functions can now be called by users
when necessary.
- Efficient hash iteration function hash_iterate allows looping over
keys and values. It is safe to delete entries while iterating (but
not insert or replace! Actually any function that calls resize is
unsafe).
freeciv_S2_1_r12321-003-hash_improvements.diff
Description: Binary data
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
