Suspicious NextIdentifier Function

Building and customizing Audacity from the source code.

If you require help using Audacity, please post on the forum board relevant to your operating system:
Windows
Mac OS X
GNU/Linux and Unix-like

Suspicious NextIdentifier Function

Permanent link to this post Posted by Edgar » Tue Sep 30, 2014 7:50 pm

The function in srccommandsCommandManager.cpp At or near line number 631:
Code: Select all
int CommandManager::NextIdentifier(int ID)
{
   ID++;

   //Skip the reserved identifiers used by wxWidgets
   if((ID >= wxID_LOWEST) && (ID <= wxID_HIGHEST))
      ID = wxID_HIGHEST+1;

   return ID;
}

looks suspicious. As of now, I don't think it's ever exercised (wxID_LOWEST Is 4999; wxID_HIGHEST Is 5999). The purpose of the function is to create and return the next unique command ID. Note that if the if clause is executed more than once (if it's going to be executed at all it will probably happen more than once) the same (non-unique) ID will be returned.

After incrementing ID (which in itself is poor programming technique because it is a parameter which is not passed by reference) there should be some kind of while loop which verifies that the new ID is in fact unique and does not impinge on the reserved identifiers. Something vaguely like:
Code: Select all
int CommandManager::NextIdentifier(int ID)
{
   int id = ID + 1;
   bool unique = IsIDUnique ();
   while (! unique) {
      //Skip the reserved identifiers used by wxWidgets
      if((id >= wxID_LOWEST) && (id <= wxID_HIGHEST))
         id = wxID_HIGHEST + id + 1;//Maybe something better?
      else
         id++;
      unique = IsIDUnique ();
   }
   return id;
}

I will leave writing bool IsIDUnique () as an exercise for the next person <grin>.
Edgar
Forum Crew
 
Posts: 1512
Joined: Thu Sep 03, 2009 9:13 pm
Operating System: Windows 7

Re: Suspicious NextIdentifier Function

Permanent link to this post Posted by Edgar » Wed Oct 01, 2014 12:04 am

I guess I ended up being the "next person"…(now) tested code:
Code: Select all
//efm5  start
bool CommandManager::IDIsUnique(int pID) {
   bool returnValue = false;
   CommandIDHash::iterator IDIterator;
   for(IDIterator = mCommandIDHash.begin(); IDIterator != mCommandIDHash.end(); ++IDIterator) {
      int commandID = IDIterator->first;
      if (pID == commandID) return false;
   }
   return true;
}

int CommandManager::NextIdentifier(int ID)
{
   //ID++;

   ////Skip the reserved identifiers used by wxWidgets
   //if((ID >= wxID_LOWEST) && (ID <= wxID_HIGHEST)) {
   //   ID = wxID_HIGHEST+1;
   //   wxMessageBox (wxT (" NextIdentifier  ID set to highest"));//efm5 debug crash
   //}

   //return ID;   
   int id = ID + 1;
   bool unique = IDIsUnique(id);
   while (! unique) {
      //Skip the reserved identifiers used by wxWidgets
      if((id >= wxID_LOWEST) && (id <= wxID_HIGHEST))
         id = wxID_HIGHEST + id + 1;//Maybe something better?
      else
         id++;
      unique = IDIsUnique(id);
   }
   return id;
   //efm5  end
}
Edgar
Forum Crew
 
Posts: 1512
Joined: Thu Sep 03, 2009 9:13 pm
Operating System: Windows 7


Return to Compiling Audacity



Who is online

Users browsing this forum: No registered users and 1 guest