[reSIProcate] removing hash<T*> from HashMap

[Byron Campen]

	Good point. I'm pretty sure that the hash table stuff uses  
std::equal (or std::less in the Windows case I suppose), and  
overriding _those_ for all types of pointers would be very bad.

	Is anyone using this?

Best regards,
Byron Campen

>
> I think there's still an underlying problem that there's a hidden  
> operator== (operator< on win32) that would be needed.  I don't know  
> of a generic way around those, although if someone else does, I  
> would love to hear about it.
>
> Bruce
>
> On Apr 23, 2008, at 4:26 PM, Byron Campen wrote:
>> 	Would it be reasonable to change the impl to call t->hash()? (I  
>> suppose we could get oddball behavior if t->hash() happened to  
>> return something interesting)
>>
>> Best regards,
>> Byron Campen
>>
>>>
>>> I would like to check in a change to HashMap to fix a problem that
>>> has occasionally bitten me in other code (not resip itself) that  
>>> uses
>>> rutil.  HashMap.hxx defines a generic hash<T*> function object that
>>> allows any code:
>>> HashMap<foo*, bar>
>>> to compile by using the address of the pointer itself as the hash.
>>> This obviously fails for any key that can exist in multiple  
>>> places in
>>> the system.
>>>
>>> This isn't an issue in resip itself as far as I can tell---in fact
>>> the only use of the hash<T*> in the code that I've been able to find
>>> is for a HashMap<pthread_t,> on macosx.  And in this case, it's easy
>>> enough to restore the previous behavior by using the HashValue
>>> macros.  But having that template in the header file allows some  
>>> code
>>> to compile that I would really not allow to compile---ever.
>>>
>>> Thoughts on whether this change would cause any problems?
>>>
>>> Bruce
>>>
>>>
>>>
>>> $svn diff
>>> Index: rutil/Log.cxx
>>> ===================================================================
>>> --- rutil/Log.cxx       (revision 7627)
>>> +++ rutil/Log.cxx       (working copy)
>>> @@ -39,6 +39,9 @@
>>>
>>>
>>>   #ifdef LOG_ENABLE_THREAD_SETTING
>>> +#ifdef __APPLE__
>>> +HashValueImp(pthread_t, (size_t)data);
>>> +#endif
>>>   HashMap<pthread_t, std::pair<Log::ThreadSetting, bool> >
>>> Log::mThreadToLevel;
>>>   HashMap<int, std::set<pthread_t> > Log::mServiceToThreads;
>>>   pthread_key_t* Log::mLevelKey = (Log::mLevelKey ? Log::mLevelKey :
>>> new pthread_key_t);
>>> Index: rutil/Log.hxx
>>> ===================================================================
>>> --- rutil/Log.hxx       (revision 7627)
>>> +++ rutil/Log.hxx       (working copy)
>>> @@ -21,7 +21,10 @@
>>>   // crashes in the Mac OS native API.
>>>   #if !defined(WIN32) && !defined(TARGET_OS_MAC)
>>>   #define LOG_ENABLE_THREAD_SETTING
>>> +#ifdef __APPLE__
>>> +HashValue(pthread_t);
>>>   #endif
>>> +#endif
>>>
>>>   namespace resip
>>>   {
>>> Index: rutil/HashMap.hxx
>>> ===================================================================
>>> --- rutil/HashMap.hxx   (revision 7627)
>>> +++ rutil/HashMap.hxx   (working copy)
>>> @@ -10,21 +10,6 @@
>>>   #    define HashMap __gnu_cxx::hash_map
>>>   #    define HashSet __gnu_cxx::hash_set
>>>
>>> -// this allows us to hash on a pointer as the key
>>> -namespace HASH_MAP_NAMESPACE
>>> -{
>>> -
>>> -template <class T>
>>> -struct hash<T*>
>>> -{
>>> -      size_t operator()(const T* t) const
>>> -      {
>>> -         return size_t(t);
>>> -      }
>>> -};
>>> -
>>> -}
>>> -
>>>   #define HashValue(type)                           \
>>>   namespace HASH_MAP_NAMESPACE                      \
>>>   {                                                 \
>>>
>>> _______________________________________________
>>> resiprocate-devel mailing list
>>> resiprocate-devel at resiprocate.org
>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2423 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20080423/a1136db5/attachment.bin>