core dump while accessing the data from STL map

[ybkumar77]

Hi,
I am writing a simple program, trying to learn the usage of map.
Following is the data, I will be storing in a map

and I get a core dump when line No: 54 is being executed.
Please let me know if something is wrong in my code.
I got struck at this point and not able to proceed further.

for the sake of keeping it simple I have removed a lot of things from the code.

class employee {
private:
          int  emp_id;
          RWCString emp_first_name;
          RWCString emp_last_name;
public:
       int getEmp_id() const {
            return emp_id;
       }
       
       const char* getEmpFirstName() const {
              return emp_first_name;
       }

       const char*  getEmpLastName() const {
             return emp_last_name;
       }
     
       void settEmp_id(int payroll_number ) const {
            emp_id = payroll_number;
       }

       void  SetEmpFirstName(RWCString tmp_fn)  {
    	       emp_first_name = tmp_fn;
       } 

       void  SetEmpLastName(RWCString tmp_ln)  {
    	       emp_last_name = tmp_ln;
       } 

     Employee * readEmpInfo(const char* nickname);
     void print_data();
     int insertEmpDataintoMap();
     static map<int, employee*> myEmpData;
};

int Employee::insertEmpDataintoMap() {
    Employee *project_lead = new Employee(101, "Bjarne","stroustroup");
    Employee *test_lead = new Employee(201, "Arthur","Cotton");
    Employee *sys_admin_lead = new Employee(212, "Billy","Joel"); 
   
    myEmpData.insert(pair<int, Employee*>(101, project_lead);
    myEmpData.insert(pair<int, Employee*>(101, test_lead);
    myEmpData.insert(pair<int, Employee*>(101, sys_admin_lead);
}

Employee * Employee::readEmpInfo(const char* nickname) {
  std::map<int, Employee*>iterator myptr;
  
  for(myptr = myEmpData.begin(); myptr != myEmpData.end(); myptr++)  {

     if (((*myPtr).second)-> getEmpFirstName() !=NULL) {
        char *compareFN=NULL;
        strcpy(compareFN, ((*myPtr).second)-> getEmpFirstName());
        if (strcmp(compareFN, nickname) == 0) {
           cout >> "found the employee you are searching for :" <<  ((*myPtr).second)-> getEmpFirstName() << endl;
        }
     }
  }
}

[IsharaComix]

Look at line 53. You've created a NULL char pointer, and then right after that you try to run a strcpy saving the data to it. That won't work since compareFN isn't pointing anywhere.

Since you're just taking a pointer, just save the return value. Try doing this instead.

if (((*myPtr).second)-> getEmpFirstName() !=NULL) {
        char *compareFN = ((*myPtr).second)-> getEmpFirstName();
        if (strcmp(compareFN, nickname) == 0) {
           cout >> "found the employee you are searching for :" <<  ((*myPtr).second)-> getEmpFirstName() << endl;
        }
     }

[mitrmkar]

The error is quite obvious

char *compareFN=NULL;
// 'compareFN' is a null pointer, strcpy() is guaranteed to fail
strcpy(compareFN, ((*myPtr).second)-> getEmpFirstName());

So strcpy() copies the source buffer to the destination buffer, which you don't have. In other words, you must have a char buffer sufficiently large to hold the incoming data + the terminating null character ('\0'). Perhaps see strcpy().

[EDIT]
Beaten by IsharaComix, providing a good suggestion, though I still suggest that you check out the strcpy().

[ybkumar77]

Hi IsharaComix,

But if you look at my getEmpFirstName() function, it returns const char*.
But by your explanation, you are suggesting me to assign const char * to char * ptr.

I think the compiler will complain about this.
let me check it out, however.

Thanks for the support,
YBKUMAR77

[ybkumar77]

Look at line 53. You've created a NULL char pointer, and then right after that you try to run a strcpy saving the data to it. That won't work since compareFN isn't pointing anywhere.

Since you're just taking a pointer, just save the return value. Try doing this instead.

if (((*myPtr).second)-> getEmpFirstName() !=NULL) {
        char *compareFN = ((*myPtr).second)-> getEmpFirstName();
        if (strcmp(compareFN, nickname) == 0) {
           cout >> "found the employee you are searching for :" <<  ((*myPtr).second)-> getEmpFirstName() << endl;
        }
     }

Hi IsharaComix,

But if you look at my getEmpFirstName() function, it returns const char*.
But by your explanation, you are suggesting me to assign const char * to char * ptr.

I think the compiler will complain about this.
let me check it out, however.

Thanks for the support,
YBKUMAR77

[ybkumar77]

Hi,
considering your suggestion, I have modified your code in the following way.

But still I get the core dump.

Employee * Employee::readEmpInfo(const char* nickname) {
 std::map<int, Employee*>iterator myptr;
 for(myptr = myEmpData.begin(); myptr != myEmpData.end(); myptr++) {
   if (((*myPtr).second)-> getEmpFirstName() !=NULL) {
      char *compareFN=new char[strlen(((*myPtr).second)-> getEmpFirstName()+1];
      strcpy(compareFN, ((*myPtr).second)-> getEmpFirstName());
      if (strcmp(compareFN, nickname) == 0) {
         cout >> "found the employee you are searching for :" <<  ((*myPtr).second)-> getEmpFirstName() << endl;

     }
   }
 }
}

[IsharaComix]

Why are you going through so much trouble just to compare the string in this manner? You're seem to be unnecessarily making life hard on yourself.

if (((*myPtr).second)-> getEmpFirstName() !=NULL) {
    if (strcmp( ((*myPtr).second)-> getEmpFirstName() , nickname) == 0) {
        cout >> "found the employee you are searching for :" << ((*myPtr).second)->getEmpFirstName() << endl;
    }
}

All you should have to do is pass the pointer. There's no reason to do any strcpy-ing.

[mitrmkar]

It looks like you are doing whole lot of extra work there. In addition the code also leaks memory, you allocate using new , but never delete that memory. Remember that you must delete the memory you have allocated.

If your RWCString class is the RogueWave's string class, then you can simplify the code a lot. That class supports e.g. operator == , so I'd suggest that you try the following and post back.

Employee * Employee::readEmpInfo(const char* nickname) {

  std::map<int, Employee*>iterator myptr;

  for(myptr = myEmpData.begin(); myptr != myEmpData.end(); myptr++) 
  {
    // temp variable 'employee' used here to make code more readable
    Employee * employee = (*myPtr).second;

    // RWCString has built-in capability to directly
    // compare against a 'const char *', so use it ...
    if(employee->emp_first_name == nickname)
    {
      cout << "found the employee you are searching for :" 
           << employee->emp_first_name << endl;
    }
  }
}

Then, would you please tell which compiler you are using? The code you've posted really should not compile in the first place. So I'm wondering whether your compiler is quite broken or are you posting somewhat irrelevant code snippets.

PS. Note that at any rate, you don't need to allocate ( new ) any memory inside that function.

[ybkumar77]

It looks like you are doing whole lot of extra work there. In addition the code also leaks memory, you allocate using new , but never delete that memory. Remember that you must delete the memory you have allocated.

If your RWCString class is the RogueWave's string class, then you can simplify the code a lot. That class supports e.g. operator == , so I'd suggest that you try the following and post back.

Employee * Employee::readEmpInfo(const char* nickname) {

  std::map<int, Employee*>iterator myptr;

  for(myptr = myEmpData.begin(); myptr != myEmpData.end(); myptr++) 
  {
    // temp variable 'employee' used here to make code more readable
    Employee * employee = (*myPtr).second;

    // RWCString has built-in capability to directly
    // compare against a 'const char *', so use it ...
    if(employee->emp_first_name == nickname)
    {
      cout << "found the employee you are searching for :" 
           << employee->emp_first_name << endl;
    }
  }
}

Then, would you please tell which compiler you are using? The code you've posted really should not compile in the first place. So I'm wondering whether your compiler is quite broken or are you posting somewhat irrelevant code snippets.

PS. Note that at any rate, you don't need to allocate ( new ) any memory inside that function.

Hi,
You are right. I am not posting the exact problem here. I think I cannot do that because of the permission issues with my client.

I am using g++
here is the version:
ivory:lib {20} > g++ -v
Reading specs from /usr/local/lib/gcc-lib/sparc-sun-solaris2.8/2.95.3/specs
gcc version 2.95.3 20010315 (release)
ivory:lib {21} >

I masked my actual problem with a miniature version of the problem (employee) for posting here. yes, I haven't compiled the code, I have posted here. I used this code only for posting here not in my project.

I am in a situation of not to disclose the problematic code and couldn't get rid of the problem either.

thanks for all the support being offered.
Regards,
ybkumar77

[ybkumar77]

Hmm... the problem got resolved. It was due to the legacy code.
in the legacy code, I was deleting the pointer which contains the address to the data
per say, I am deleting the employee after using it from the below code.

Employee * employee = (*myPtr).second;

and in the next function call, I am trying to access the already deleted data. that's what is causing the problem.

thanks IsharaComix and Mitrmkar for your suggestions and bearing my foolishness :)

Regards,
ybkumar