[Ace-users] [ace-users] Strange code in ACE_Hash_Multi_Map_Manager

[Johnny Willemsen]

Hi Boris,
 
Thanks for using the PRF form. Can you put this in bugzilla for integration,
also add a unified diff (diff -u) of the change.
 
Regards,
 
Johnny Willemsen
Remedy IT
Postbus 101
2650 AC  Berkel en Rodenrijs
The Netherlands
www.theaceorb.nl / www.remedy.nl 

*** Integrated compile and test statistics see
http://scoreboard.theaceorb.nl <http://scoreboard.theaceorb.nl/>  ***
*** Commercial service and support for ACE/TAO/CIAO             ***
*** See http://www.theaceorb.nl/en/support.html                 *** 


________________________________

	From: ace-users-bounces at cse.wustl.edu
[mailto:ace-users-bounces at cse.wustl.edu] On Behalf Of Boris Kaminer
	Sent: Friday, December 14, 2007 7:34 AM
	To: ace-users at cse.wustl.edu
	Subject: [ace-users] Strange code in ACE_Hash_Multi_Map_Manager
	
	

	    ACE VERSION: 5.6.1 

	    AREA/CLASS/EXAMPLE AFFECTED: ACE_Hash_Multi_Map_Manager

	 

	    DOES THE PROBLEM AFFECT:

	        EXECUTION?  - YES

	 

	    SYNOPSIS:

	Redundant copying in bind_i method

	 

	    DESCRIPTION:

	The code below presented in ACE_Hash_Multi_Map_Manager at least
since 5.5.3.

	 

	template <class EXT_ID, class INT_ID, class HASH_KEY, class
COMPARE_KEYS, class ACE_LOCK> int

	ACE_Hash_Multi_Map_Manager<EXT_ID, INT_ID, HASH_KEY, COMPARE_KEYS,
ACE_LOCK>::bind_i (const EXT_ID &ext_id,

	
const INT_ID &int_id,

	
ACE_Hash_Multi_Map_Entry<EXT_ID, INT_ID> *&entry)

	{

	  size_t loc;

	  int result = this->shared_find (ext_id, entry, loc);

	 

	  ACE_Unbounded_Set<INT_ID> int_id_set;

	  if (result == -1)

	    {

	      void *ptr;

	      // Not found.

	      ACE_ALLOCATOR_RETURN (ptr,

	                            this->entry_allocator_->malloc (sizeof
(ACE_Hash_Multi_Map_Entry<EXT_ID, INT_ID>)),

	                            -1);

	 

	      int_id_set.insert (int_id);

	 

	      entry = new (ptr) ACE_Hash_Multi_Map_Entry<EXT_ID, INT_ID>
(ext_id,

	
int_id_set, ß Unnecessary copying

	
this->table_[loc].next_,

	
&this->table_[loc]);

	      this->table_[loc].next_ = entry;

	      entry->next_->prev_ = entry;

	      this->cur_size_++;

	      return 0;

	    }

	  else

	    {

	     // Just unbelieveable way of new element inserting

	      int_id_set = (*entry).int_id_set_;

	 

	      if (0 == int_id_set.insert (int_id))

	        {

	          this->unbind_i (entry);

	          return this->bind_i (ext_id, int_id_set);

	        }

	      else

	        return 1;

	    }

	}

	 

	    REPEAT BY:

	[What you did to get the error; include test program or session

	transcript if at all possible.  ]

	 

	    SAMPLE FIX/WORKAROUND:

	This is a straightforward fix, but more correct implementation
requires introducing an additional constructor for ACE_Hash_Multi_Map_Entry
without

	const ACE_Unbounded_Set<INT_ID> &int_id_set parameter and using it
during an entry creation.

	 

	template <class EXT_ID, class INT_ID, class HASH_KEY, class
COMPARE_KEYS, class ACE_LOCK> int

	ACE_Hash_Multi_Map_Manager<EXT_ID, INT_ID, HASH_KEY, COMPARE_KEYS,
ACE_LOCK>::bind_i (const EXT_ID &ext_id,

	
const INT_ID &int_id,

	
ACE_Hash_Multi_Map_Entry<EXT_ID, INT_ID> *&entry)

	{

	  size_t loc;

	  int result = this->shared_find (ext_id, entry, loc);

	 

	  if (result == -1)

	    {

	      void *ptr;

	      // Not found.

	      ACE_ALLOCATOR_RETURN (ptr,

	                            this->entry_allocator_->malloc (sizeof
(ACE_Hash_Multi_Map_Entry<EXT_ID, INT_ID>)),

	                            -1);

	 

	      entry = new (ptr) ACE_Hash_Multi_Map_Entry<EXT_ID, INT_ID>
(ext_id,

	
ACE_Unbounded_Set<INT_ID>(), 

	
this->table_[loc].next_,

	
&this->table_[loc]);

	      this->table_[loc].next_ = entry;

	      entry->next_->prev_ = entry;

	    }

	 

	   if (!(*entry).int_id_set_.insert(int_id)) {

	      this->cur_size_++;

	      return 0;

	   } else

	        return 1;

	}

	 

	 

	/Boris