[Aptitude-devel] Review of 13edae

[Daniel Burrows]

On Mon, Aug 02, 2010 at 05:06:56PM -0700, Daniel Burrows <dburrows at debian.org> was heard to say:
> >  	boost::unordered_map<tab_widget *, boost::unordered_set<packages_tab *> > active_packages_tabs;
> >  
> > -	/** \brief List of all opened package_info tabs.
> > -	 *   \todo shouldn't this be unordered_map?
> > -	 */
> > -	boost::unordered_set<package_info_tab *> package_info_tabs;
> > +	/** \brief List of all opened package_info tabs. */
> > +	boost::unordered_map<package_ptr, package_info_tab *> package_info_tabs;
> 
>   Are you sure this is going to be effectively hashed?  I think you
> might need a custom hash function.  We could probably drop a generic
> one in src/generic/util/hash_smart_ptrs.h, something like this:

  On further reflection, there's a deeper problem here.  You're storing
package_ptr objects, pointers to things managed by the package pool.
These uniquely identify a package while the cache is loaded, but if it
gets reloaded, new package_ptr objects are created for the new cache.

  Really, you want the info tabs to be unique per package *name*.  I
suggest just using the name as the key for the info tab list (since
std::string has a built-in hasher, this also means you don't have to
write a custom hasher).

  Daniel