Using struct and point to input and output info

Question

I have a problem with inputting a string pointer in struct. This is my code:

typedef struct{
        char *name;
        int age;      
}stu;

void allocate(stu* &s, int n){
     s = (stu*) malloc(n * sizeof(stu));
     if(s == NULL){
          printf("\nNot enought memory!");
          exit(1);
     }     
}
// Input info
void input_info(stu* &s, int n){
     void input(stu &s); //prototype
     for(int i = 0; i < n; i++){
             printf("\n-- Student #%d:", i+1);
             input(*(s+i));
     }    
}

void input(stu &s){
     fflush(stdin);
     printf("\nEnter student's name: ");
     gets(s.name);
     printf("\nEnter student's age: ");
     scanf("%d", &s.age);
}
// End input

//Output info
void output_info(stu* s, int n){
     void output(stu s); //prototype
     for(int i = 0; i < n; i++){
             printf("\n-- Student #%d:", i+1);
             output(*(s+i));
     }
}

void output(stu s){
     printf("\nName: %s", s.name);
     printf("\nAge: %d", s.age);
}
//End output

int main(){
    stu* s;
    int n;
    printf("How many students you want to input?: ");
    scanf("%d", &n);
    allocate(s, n);
    input_info(s, n);
    output_info(s, n);
    getch();
}

When I input second student's name, it's breaked? I allocated memory. And I want to ask how to deallocate memory for stu pointer? Thanks for reading

[Read a book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list), because this is not proper C++ code. Use std::string, don't ever touch malloc, remove all pointers and dynamic allocation, don't use stdio. — Cat Plus Plus, Mar 31 '13 at 12:56
Adding salt into that wound.. `fflush(stdin);` is undefined by the standard. — WhozCraig, Mar 31 '13 at 12:57
As Cat Plus Plus said, your code is very unidiomatic. Are you trying to write C++ code or C code ? It looks like C code but with a few C++ features mixed in. The solution won't be the same depending on your goal. — Fabien, Mar 31 '13 at 12:58
The oddest part to me is amongst this chaos he actually uses a reference-to-pointer parameter *correctly* (albeit his stock immediately goes down with the ensuing `malloc()`). I know professional engineers that struggle with that concept, and he just walks up and tags it =P — WhozCraig, Mar 31 '13 at 13:01
See that `gets()`? That thing is so evil its deprecated and won't e in the next version of the language. Think about what it is doing, where it is *putting* the data it is supposedly getting, and how you can avoid that mess by using `std::string`, `std::getline()` and throwing out that `malloc()` in favor of `new`. — WhozCraig, Mar 31 '13 at 13:05
@WhozCraig Moreover, it's the only C++ feature in this code :) — SomeWittyUsername, Mar 31 '13 at 13:21
Oh, I sorry. I code it with C but I coded it with a few C++ >" — mthe.net, Mar 31 '13 at 15:01

score 4 · Answer 1 · edited May 23 '17 at 10:25

There are many things that can and should be improved in your program. Some suggestions:

Remove the char * member replace it with std::string. You don't want to do manual memory management for anything unless you really want to.
Drop the silly scanf and printf, they are not type safe. Since you are using C++ use std::cin and std::cout and you are much safer than usage with later.
Drop the fflush(stdin), Calling fflush on anything other than stdout gives you undefined behavior.
In C++, You would usually want to use new and not malloc. Avoid using dynamic memory allocation at all, if you can. Prefer to use a std::vector instead.

Online Sample:
Following the above suggestions your example can be written as:

#include<string>
#include<vector>
#include<iostream>

typedef struct
{
    std::string name;
    int age;      
}student;

// Input info
void input_info(std::vector<student> &s)
{ 
    student obj;
    std::cout<<"\nEnter Students name";
    std::cin>>obj.name;
    std::cout<<"\nEnter Students age";
    std::cin>>obj.age;
    s.push_back(obj);
}

// Output info
void output_info(const std::vector<student> &s)
{
    for (auto itr = s.cbegin(); itr != s.cend(); ++itr)
    {
        std::cout<<"\nName:"<< itr->name;
        std::cout<<"\nAge:"<< itr->age;
    }
}

int main()
{
    int n;
    std::cout<<"How many students you want to input?\n";
    std::cin>>n;
    std::vector<student>s;
    for(int i = 0; i<n; i++)
    {
        input_info(s);
    }
    output_info(s);
    return 0;        
}

Also, as @BLUEPIXY points out, no space has been allocated for `name`, so writing to it is undefined behavior. — Beta, Mar 31 '13 at 13:24
Yes, but it's an important error to point out for pedagogy, if only to drive #1 home. — Beta, Mar 31 '13 at 13:40

score 0 · Answer 2 · edited May 23 '17 at 10:32

As some of the previous poster have mentioned, the code is not "pure" c++, as it mixes a lot of C and C++ features. Personally I think it is a less problematic when dealing with POD structs, like yours.

The crash is likely caused by gets(). It assumes that the char pointer is already allocated with a suitable capacity. It has undefined behavior when the inputs strings is longer than the capacity. Your capacity is 0, hence the crash.

If you insist on using C functions, see: Safe Alternative to gets. Otherwise lookup getline().

Using struct and point to input and output info

2 Answers2