[Aptitude-devel] Review of 13edae

[Piotr Galiszewski]

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).
>
>  Daniel
>

You are right. I have not tested this particular case. Will fix this

> _______________________________________________
> Aptitude-devel mailing list
> Aptitude-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/aptitude-devel
>
-- 
Regards
Piotr Galiszewski