Assigning a value to char* in a struct

Question

I have problem with my code i cant assign a string value into char* in a struct. Can someone tell me what is wrong with my code and why?

#include <iostream>
using namespace std;

typedef struct{
char* name;
char* city;
int age;
} person;
void main()
{
person * user;
user = (person*)malloc(sizeof(person*));

cout << "Please fill in the user info.." << endl << "Name: ";
cin >> user->name;
cout << "Age: ";
cin >> user->age;
cout << "City";
cin >> user->city;

cout << "The user info is:" << endl << "Name: " << user->name << endl << "Age: " << user->age << endl << "City: " << user->city << endl;
system("pause");
}

Thank you very much.

Answer 1

Your code is a horrible mix of C and C++ style, as Mike's comment says, pick a language and use it properly.

#include <iostream>
#include <string>
using namespace std;

struct person {
  string name;
  string city;
  int age;
};

int main()
{
  person user;

  cout << "Please fill in the user info.." << endl << "Name: ";
  cin >> user.name;
  cout << "Age: ";
  cin >> user.age;
  cout << "City";
  cin >> user.city;

  cout << "The user info is:\n" << "Name: " << user.name << "\nAge: " << user.age << "\nCity: " << user.city << endl;
  system("pause");
}

Don't dynamically allocate when you don't need to (then there's no risk of mallocing the wrong amount of memory, and no risk of not mallocing memory for the strings, which are both mistakes you made in your original program).

main must return int not void

typedef struct {...} x; is not necessary in C++, just say struct x {...};

Don't overuse endl

Answer 2

You've tagged the question with two different languages, and seem to be coding in a ghastly blend of the two. You should choose one language and stick to that.

If this is meant to be C++, then use the standard library:

#include <string>
#include <iostream>

struct person {
    std::string name;
    std::string city;
    int age;
};

int main() {    // not void
    person user;
    // ...
    std::cin >> user.name;
    // ...
}

If it's meant to be C, then you need to allocate memory for the strings. Either allocate them from the heap:

person user;
user.name = malloc(MAX_NAME_SIZE);
user.city = malloc(MAX_CITY_SIZE);

or embed them in the structure:

typedef struct {
    char name[MAX_NAME_SIZE];
    char city[MAX_NAME_SIZE];
    int name;
} person;

and take care not to overflow these fixed-size buffers when you read from the input.

If you really want to use malloc for the structure itself for some reason, then allocate enough for the structure, not just a pointer:

user = malloc(sizeof(person));

Answer 3

malloc(sizeof(person));

This would create memory for holding 2 char* and 1 int.

You also need to allocate memory for char*s in struct which you missed.

Answer 4

I don't know about c++, but if you choose to stick to c, your user->name is not allocated memory, neither user->city. You need to use malloc() with proper size to allocate memory to the pointer variable, then use those to store the values.

Also,

user = (person*)malloc(sizeof(person*));

should be

user = malloc(sizeof(*user));

Then , void main() should be changed to int main() and need to add return 0. It's a better practice to send back the exit status to the shell.

---

Note: by seeing the usage of

#include <iostream>
using namespace std;

it looks more of a c++ thing, but IMO, the logic should be same.