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

[Byron Campen <bcampen@xxxxxxxxxxxx>]

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

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