[Aptitude-devel] Review of 13edae
piotr at galiszewski.pl
Tue Aug 3 18:42:22 UTC 2010
2010/8/3 Daniel Burrows <dburrows at debian.org>:
> 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).
You are right. I have not tested this particular case. Will fix this
> Aptitude-devel mailing list
> Aptitude-devel at lists.alioth.debian.org
More information about the Aptitude-devel