Exception in homemade string class

Question

I wrote a C++ class named DCstring similar to String class in Java.

class DCstring
{

public:
     TCHAR  *string;
     DCstring();
     DCstring(TCHAR * str);
          DCstring in();
}
DCstring::DCstring()
{
    string=NULL;

}
//constructor to initialize
DCstring::DCstring(TCHAR* temp)
{
    _TINT i=0;
    string=(TCHAR*)malloc(_tcslen(temp)*sizeof(TCHAR)+1);
    for(;i<_tcslen(temp);i++)
        string[i]=temp[i];
    string[i]=0;
}
//to get input from user
DCstring DCstring::in()
{
    wprintf(L"\n Enter the string :");
    TCHAR arr[200],*s;
    _getts(arr);
    _TINT i=0;
    string=(TCHAR*)realloc(string,_tcslen(arr)*sizeof(TCHAR)+1);
    for(;i<_tcslen(arr);i++)
        string[i]=arr[i];
    string[i]=0;
    return(*this);
}

This works fine. I use this DCstring variable inside a struct and reads & writes that struct object into &from a file using fwrite & fread functions.

typedef struct Stud
{
    DCstring name;
}stud;

void InsertToFile(stud *temp,FILE *file)
{
    (temp->name)=DCstring();
    fflush(stdin);
    temp->name=(temp->name).in();
        fseek(file,0,SEEK_END);
    fwrite(&(*head),sizeof(stud),1,file);
}

void Show(stud *temp,FILE *file)
{
        rewind (file ) ;
        fread ( &temp,sizeof(stud), 1, file ) ;
        wprintf ( L"\n%s",temp->name ) ;
}

This code can read and write data for the 1st time.when I reexecute the code and call Show(stud *temp,FILE *file) function,it throws an runtime error/exception.

"Unhandled exception at 0x77c815de in StudentRecord.exe: 0xC0000005: Access violation reading location 0x002850a8".

seems like problem in memory.Help me out.

Have you tried using a debugger? That is what it's for, after all... In fact scratch that, why aren't you using `std::string`? — trojanfoe, Jul 08 '13 at 08:51
I may be out of topic but when I saw your code the first thought I had was: wouldn't you use `std::string` instead? — gx_, Jul 08 '13 at 08:54
show the default constructor of DCstring, it will be called in the struct. the line `(temp->name)=DCstring();` doesn't make any sense. how do you call the Show function? maybe with a NULL pointer? — WoJo, Jul 08 '13 at 09:05
The class violates [the rule of three](https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)) and therefore leaks memory. — Cody Gray - on strike, Jul 08 '13 at 09:10

trojanfoe · Answer 1 · 2013-07-08T09:40:34.263

2

Apart from the poor implementation of your DCString class (exposed instance variables, lack of destructor, fixed-sized input buffer, and some more) you have failed to understand that writing an instance of your struct stud won't, in fact, write the contents of the data pointed to by stud.name.string.

struct stud contains an instance of DCString however the memory used to store the string data is on the heap (which you allocated using malloc()); your code is writing the pointer to the allocated memory, which will be meaningless when you read it back in (certainly in a different session).

You need to provide a read/write DCString method, which is passed a FILE * (better would be to use istream and ostream, given this is C++) which reads/writes the string contents, allocating new memory during the read.

edited Jul 08 '13 at 09:40

answered Jul 08 '13 at 09:05

trojanfoe

120,358
21
212
242

thanks trojanfoe. would replacing *string with string[200] and change in() method solves problem??? – krish Jul 08 '13 at 09:59
@krish No, I wouldn't think so. You should consider dropping your `DCString` class in favour of `std::string` and reconsider your read/write code completely. Remember you cannot write objects to files like that and expect them to work when you read them; you need to tell them to write themselves or provide a method to do it for them. – trojanfoe Jul 08 '13 at 10:06

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

1

The problem is here:

public:
    TCHAR  *string;

This writes out a pointer to the string, so when you read back in you get a pointer that doesn't point to any data, thus your access violation. This question, for instance, discusses serialisation, which is what you probably want to do.

Furthermore:

fflush(stdin);

Is undefined behaviour.

Yet furthermore, you should really use C++ std::string and std::i/ofstream.

EDIT I just noticed:

wprintf ( L"\n%s %d %c",temp->name ) ;

You don't have anything matching the %d and %c parameters.

edited May 23 '17 at 10:32

Community

1
1

answered Jul 08 '13 at 08:57

Ken Y-N

14,644
21
71
114

No, that is NOT his problem. – trojanfoe Jul 08 '13 at 09:01
I added an extra note about the extraneous `printf` parameters, if that is what you are referring to. – Ken Y-N Jul 08 '13 at 09:07
Nope; look at what the `fread()` and `fwrite()` functions are doing and think about the implications. – trojanfoe Jul 08 '13 at 09:08
Heh, my eyes glazed over when I first looked at them: if we assume `head` is meant to be `temp` in `InsertToFile`, then the `fwrite` is OK, but `fread ( &temp,sizeof(stud), 1, file ) ;` should not have the `&` before `temp`. Thanks for pointing that out! – Ken Y-N Jul 08 '13 at 09:29
Sorry, not even that. As mentioned in my answer, if the OP is hoping to read and write the string contents then simply writing the object to file won't do the job. Not only is it bad practise and non-portable, it simply won't work as the string contents is actually on the heap and all `fwrite()` will write is the pointer which is meaningless when read back in. – trojanfoe Jul 08 '13 at 09:32

Exception in homemade string class

2 Answers2