What's wrong with this strcpy and this pointer

[earlyriser]

Hello
I'm trying to assign a value from a string via some pointer, but I got an error and I cannot figure how to solve it.
Do you have an idea? I would really help me. The problematic line is marked between *********
Thanks.

// reading a text file
#include <iostream>
#include <fstream>
#include <string>
using namespace std;


struct Cours
{
    char *sigle;
    Cours *suivant;
};

struct Etudiant
{
    char *nom;
    Etudiant *apres;
};

struct Professeur
{
    char nom [20];
    Professeur *suivant;
};


class DossierProfesseur
{        
    public:
        Professeur *tete;
        DossierProfesseur();
        ~DossierProfesseur();
        void affichage();
};


DossierProfesseur::DossierProfesseur()
{
    Professeur *PCourant, *PTete;
    string line;
    ifstream myfile ("FP.txt");
    if (myfile.is_open())
    {
        while (! myfile.eof() )
        {
            getline (myfile,line);
            cout << line << endl;
            PCourant = new Professeur;
/*****************************************************/
            strcpy (PCourant->nom, line);
/*****************************************************/
        }
        myfile.close();
    }

    else cout << "Unable to open file";
}


void main () {
 

    DossierProfesseur *dossierProfesseur;
    
    dossierProfesseur= new DossierProfesseur;
}

[iamthwee]

http://www.cplusplus.com/reference/clibrary/cstring/strcpy.html

[earlyriser]

Thanks iamthwee, but it is not helping me a lot, I was in that page before writing this post and I did what I understood:

strcpy (where-the-data-will-be, where-there-are-now)

strcpy (PCourant->nom, line);

[Ancient Dragon]

>> strcpy (PCourant->nom, line);

line is a c++ class, strcpy() wants a c style pointer. You have to use std::string's c_str method to get the pointer, like this
strcpy (PCourant->nom, line.c_str());

struct Professeur
{
    char nom [20];
    Professeur *suivant;
};

Since you are writing a c++ program you should use c++ std::string class, not character arrays, unless your teacher requires you to use character arrays. Using std::string will make your life a lot easier and eleminate several problems with using strcpy() and other similar standard c string handling functions. The main problem with those functions is they will allow you to scribble all over memory by copying strings beyond the allocation of the destination buffer. Example: copy a 25 character string into that buffer that has only room for 19 characters plus null terminator. After that happens your program will likely crash at some point and you will spend hours trying to find the problem.

[iamthwee]

yeah and don't forget to clean up after with delete[], avoid void main and EOF blah blah blah.

[earlyriser]

Thanks Ancient Dragon!
It works, well, the program doesnt, but at least the strcpy works.
My teacher wants char arrays.

Thanks iamthwee!

[earlyriser]

After reading a line in a text file I need to read another line which contains an INT. I thought that the bold line in this code will be ok to retreive the data, but it doesnt.

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

struct Professeur
{
    char nom[20];
    int ancien;
};


class DossierProfesseur
{
   
        
    public:
        Professeur *tete;
        DossierProfesseur();
        ~DossierProfesseur();
};


DossierProfesseur::DossierProfesseur()
{
    Professeur *PCourant, *PTete=NULL;
    string line;
    ifstream myfile ("FP.txt");
    if (myfile.is_open())
    {
        while (! myfile.eof() )
        {
            getline (myfile,line);
            PCourant = new Professeur;
            strcpy (PCourant->nom, line.c_str());
            cout << PCourant->nom;
            <strong>getline (myfile, PCourant->ancien);</strong>
        }
        myfile.close();
    }

    else cout << "Unable to open file";
}


void main () {
 

    DossierProfesseur *dossierProfesseur;
    
    dossierProfesseur= new DossierProfesseur;
}

[John A]

You're trying to use getline() to read a string into an integer. It just ain't going to work. Read it into a string, and take it out with atoi() or use a stringstream.

std::string line;
std::stringstream lineStream;
getline(myFile, line);
lineStream << line;
lineStream >> PCourant->ancien;

Your code is driving me crazy! If you're going to use char arrays, you might as well be using C. And no, I know it's not your fault, so I blame your teacher for that.

[earlyriser]

Thanks joeprogrammer!
I will try that and give the feedback tomorrow, (I have been doing this homework all the day).
I didn't know that C is better for char arrays than C++.

[John A]

Thanks joeprogrammer! I will try that and give the feedback tomorrow, (I have been doing this homework all the day). I didn't know that C is better for char arrays than C++.

It's better because C functions were designed for char arrays. A lot of C++ functions were designed for std::string. You could always use the old C functions to manipulate the mix of C and C++ strings, but you usually shouldn't mix C and C++ functions unless it's absolutely necessary.

[Ancient Dragon]

I didn't know that C is better for char arrays than C++.

That's not true, character arrays can be handled equally by either language.

[John A]

That's not true, character arrays can be handled equally by either language.

Well, yes.

My point was that it's like this. Say you want to input a string using getline():

#include <string>
std::string line;
getline(cin, line);

Now that's all fine and dandy. But let's say you want to copy this to a char string:

#include <cstring>
char name[20];
strcpy(name, line.c_str()); // should error check this

But now you've just had to resort to C to process the string. Sure there are probably workarounds, but generally it's a much, much better idea to use std::strings (although not in all cases; for example some constructors such as std::stream require C-strings).

[WaltP]

If you're going to use char arrays, you might as well be using C. And no, I know it's not your fault, so I blame your teacher for that.

Seems to me the concept here is to learn to process c-strings, because they are necessary in C++.earlyriser, stop using any and all functions that have a definition of string and use only char* functions. Help us save Joe's sanity. :)

[Ancient Dragon]

Seems to me the concept here is to learn to process c-strings, because they are necessary in C++.

earlyriser, stop using any and all functions that have a definition of string and use only char* functions. Help us save Joe's sanity. :)

He's already said his instructor says he must use char arrays. He should be writing a pure C program, not a C++ program basterdised with C. If that is what his instructor is teaching then the instructor needs to take a few more programming courses.

[earlyriser]

Hi. In fact the Homework is in C++.
Maybe the error is mine (I dont want to embarras my teacher's reputation).
The structs provided in the HW are below, with the chnages I did because I wnated to reduce the difficulty and have less pointers. Maybe that was the thing that brought the insanity.

struct  Professeur{
   char*   nom; // <strong>changed to char nom[20]</strong>
   char*   prenom;
   Cours*  listecours;
   Etudiants*  listetudiants;
   Professeur* suivant;
};
 
struct Cours {
   char     * sigle; // <strong>changed to char sigle[10]</strong>
   Cours  * suivant;
};
 
struct  Etudiant {
   char*     nom; //changed to char nom[20]
   Etudiant* apres;
};

By the way the point of the HW are classes and not the string manipulation

[iamthwee]

>If that is what his instructor is teaching then the instructor needs to take a few more programming courses.

Not really, like Mr disney said it's nice to know about char[] style strings. In fact you'll find a lot of institutions teach kids c++ this way.

Will they come out knowing true c++? Nup, but there ya go. :lol:

[Ancient Dragon]

>>By the way the point of the HW are classes

Well, so much for your teacher's reputation, it just went into the gutter :eek: But we should be discussing your HW, not your teacher.

Had you left the pointers there it would have made your program just a small tad more complex but a whole lot safer because you could have allocated memory of the correct size for the strings, something like this:

PCourant->nom = new char[line.size()+1];
 strcpy (PCourant->nom, line.c_str());

Don't forget to delete[] to memory when done with it.

[earlyriser]

Thanks for all your tips. I will close this thread, because I have started another one with a less specific topic:
http://www.daniweb.com/techtalkforums/post317987.html#post317987

[Lazaro Claiborn]

After reading a line in a text file I need to read another line which contains an INT. I thought that the bold line in this code will be ok to retreive the data, but it doesnt.

#include <iostream>
#include <fstream>
#include <string>
#include <cstdlib>
using namespace std;
 
struct Professeur
{
    char nom[20];
    int ancien;
};
 
 
class DossierProfesseur
{
 
 
    public:
        Professeur *tete;
        DossierProfesseur();
        ~DossierProfesseur();
};
 
 
DossierProfesseur::DossierProfesseur()
{
    Professeur *PCourant, *PTete=NULL;
    string line;
    ifstream myfile ("FP.txt");
    if (myfile.is_open())
    {
        while (! myfile.eof() )
        {
            getline (myfile,line);
            PCourant = new Professeur;
            strcpy (PCourant->nom, line.c_str());
            cout << PCourant->nom;
            <strong>getline (myfile, PCourant->ancien);</strong>
        }
        myfile.close();
    }
 
    else cout << "Unable to open file";
}
 
 
void main () {
 
 
    DossierProfesseur *dossierProfesseur;
 
    dossierProfesseur= new DossierProfesseur;
}

Being the nerd I am, I'll give my opinion on this particular issue. If you're going to use a structure data type, I'd just use malloc instead of new to allocate memory on the heap:

PCourant = (struct Professeur*)malloc(sizeof(struct Professeur));

Or you can define a macro something similar to new such as:

#define Professor Professeur
#define new(x) (x*)malloc(sizeof(x))
 
PCourant = new(Professor);

And for the string variable line, you might as well use a character array of say 100 characters or etc:

#define MAX_LINE 100
char line[MAX_LINE];

This will save the trouble calling the c_str() string class function and perhaps a bit of proccessor time. So now you previous version would be ok. Last, don't forget to free the allocated memory via the "free" function.

This is just an alternative way, but I guess it boils down to the specifications of your assignment.

Good luck, Lamabot

[John A]

Being the nerd I am, I'll give my opinion on this particular issue. If you're going to use a structure data type, I'd just use malloc instead of new to allocate memory on the heap:

PCourant = (struct Professeur*)malloc(sizeof(struct Professeur));

Why would you want to do that? :rolleyes:

This is a C++ program. So then use C++ memory allocation methods. The OP's method of using new was fine. (Aside: 2 things the OP did do wrong; didn't use delete and there's a memory leak in the loop as I mentioned previously.)And for the string variable line, you might as well use a character array of say 100 characters or etc:

#define MAX_LINE 100
char line[MAX_LINE];

Yeah, except that getline() requires a C++ string as an argument... that's why the OP used it in the first place.

And while I'm critcizing people's code (sorry, I can't help it!), trash that void main(). It's not standard, nor is it good practice. http://users.aber.ac.uk/auj/voidmain.shtml