1

I have got class "student.cpp"

#include <iostream>
#include "student.h"
using namespace std;


void student::setMarks(int m1, int m2) {
    mark1 = m1;
    mark2 = m2;
};
void student::setName(char *n) {
    name = n;
};
int student::calc_media(void){
    return (mark1+mark2)/2;
};
void student::disp(void){
    cout << "Student:" << name << " \n media:"<< calc_media() <<"\n";
};

student::student(){
   mark1 = 0;
   mark2 =0;
   name = "";
};

Header file "student.h":

ifndef CLASY_STUDENT_H
#define CLASY_STUDENT_H

#endif //CLASY_STUDENT_H

class student{

    char *name;
    int mark1, mark2;

public:
    void setName(char *n);
    void setMarks(int m1, int m2);
    void disp(void);
    int calc_media(void);
    student();
};

And "main.cpp":

#include <iostream>
#include "student.h"

using namespace std;

int main() {
    student s;
    char* n;
    int m1, m2;

    cout << "Enter name:";
    cin>> n;
    cout << "Enter marks of two subjects:";
    cin>> m1;
    cin>> m2;

    s.setName(n);
    s.setMarks(m1, m2);

    s.disp();
    return 0;
}

I am running this usign Clion and Cmake is :

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall")

set(SOURCE_FILES main.cpp student.cpp student.h student.cpp student.h)

But when I run, it asks for name, but when I type something then I have got a memory fragmentation error. Whats wrong? And could someone by the way tell me if it looks okey for C++? I am trying to switch from java to c++.

Vertexwahn
  • 7,709
  • 6
  • 64
  • 90
Mateusz
  • 604
  • 8
  • 20

2 Answers2

6
char* n;
...
cin>> n;

n is a pointer, supposed to point at a particular piece of memory. But you never set it. So it has some undefined value, pointing who-knows-where into some memory that you end up trying to overwrite. Most likely memory you are not allowed to overwrite, causing a segfault.

Don't try to use char* if you don't yet know about manual memory management (and once you do, you'll understand why not to). Use std::string.

From a quick glance, you can pretty much replace char* everywhere with std::string (as long as you #include <string>).

BoBTFish
  • 19,167
  • 3
  • 49
  • 76
  • I thought that it can be wrong but I was learning with tutorial on erlerobotics and here it works: https://erlerobotics.gitbooks.io/erle-robotics-cpp-gitbook/content/code/7.Classes/e_7.3.cpp – Mateusz Mar 01 '16 at 14:30
  • 1
    @Mateusz Your tutorial might be bad then. We have a list of [suggested books](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – BoBTFish Mar 01 '16 at 14:31
  • @Mateusz I'm not going to download that at work, but from the title, I assume it is targeted at embedded systems for robotics. This is often a limited-memory world, where you do things in a way that is not good practice in general. – BoBTFish Mar 01 '16 at 14:32
  • Ok, it is this book if you would like to see later: https://erlerobotics.gitbooks.io/erle-robotics-cpp-gitbook/content/classes_and_structs/exercises_classes.html – Mateusz Mar 01 '16 at 14:34
  • but is there a possibility to use "cin"' and save it in char pointer? – Mateusz Mar 01 '16 at 14:39
  • 1
    @Mateusz It is technically possible, but there has to be some allocated memory at the other end of the pointer. And offhand, I can't remember exactly how you stop somebody with a really long name overflowing the memory you allocate - which implies it is fiddly, and a good reason not to do it. [`std::setw`](http://en.cppreference.com/w/cpp/io/manip/setw), I think. – BoBTFish Mar 01 '16 at 14:42
1

Similar to what others are saying, your variable n is an uninitialized pointer. Pointers, as the name suggests, are just signposts to a particular location in memory - the tell the CPU "go to this memory location for variable x".

Say you have an integer variable var, which is declared like this:

int var;

That variable occupies memory and you can assign it a value like this:

var = 5;

You can also declare a pointer to an integer like this:

int * var_ptr;

Now assuming var_ptr points to a valid integer I can assign a value to it like this:

*var_ptr = 5;

This says "put the number 5 at the memory location pointed to by var". However if var_ptr has not been initialized then it will point to a random location in memory that may overwrite something important, or attempt to write to a protected memory address causing a protection fault (segfault). This is what is happening in your code.

To initialize var_ptr to point to the address of var, we can do this:

var_ptr = &var;

The ampersand is the "address of" operator - it says "don't get me the value of var but instead get me the address of the memory location where var is stored".

So, to prevent your problem, you must initialize n to some valid memory location where you are able to safely write some data.

There are a few ways to do this. As @Stefan points out you can declare n to be a character array:

char n[20];

As @BobTFish points out you need some way to make sure that your input doesn't exceed the size of your array (20 bytes in this case). The solution is std::cin.width(20).

As @BobTFish also mentions, you could also using a std::string, like this:

std::string n;
std:cin >> n;

The std::string object will automatically take care of memory allocation.

If you really must use a char *, you can either take the address of a char array (here I take the address of the first element of the array):

char n_array[20];
char *n = &n_array[0];
std::cin.width(20);
std::cin >> n;

You could also use dynamic memory allocation:

char *n = new char[20];
std::cin.width(20);
std::cin >> n;
delete n;

Notice that if you use dynamic memory allocation you must free the memory using delete when you are done otherwise there will be a memory leak. Local variables (like the array) are allocated on the stack and therefore are automatically freed when the function returns. For this reason, and the overhead of dynamic memory allocation, you would be insane to use it here.

Trevor
  • 311
  • 2
  • 6