Re: [reSIProcate] removing hash<T*> from HashMap

[Byron Campen <bcampen@xxxxxxxxxxxx>]

	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@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel

Attachment: smime.p7s
Description: S/MIME cryptographic signature