c++ newb help

Cant get this to work anyone see something i dont . it compiles w/no errors borland but will not run .


#include <iostream.h>
#include <string.h>

class automobile{
private:
char *make, *model, *color, *engine;
public:
void set_values (char *, char *, char *, char *);
void display() {cout << make<< model<< color<< engine;}
};

void automobile::set_values (char* a,char* b,char* c,char* d)
{
strcpy(make, a);
strcpy(model, b);
strcpy(color, c);
strcpy(engine, d);
}
void main(){

automobile car;
car.set_values ("honda", "accord", "blue", "v6");
car.display();


}

Been a really long time since i have messed with C++, so my apologies if this is from far left field, but isn't main supposed to be an int with some args? I think I remember using void in the past had caused me problems...

Try something like:

int main(int argc, char *argv)


Saw an interesting discussion about this after doing a google search for 'int main':

http://www.eskimo.com/~scs/readings/...in.960823.html

/nebulus

thanks for trying nebulus, but i dont need my main funtion to return anything .

That doesn't necessarily matter. I remember having problems with programs using the Bordland compiler and using void for main not running no matter what and changing it to int seemed to fix the problem.

/nebulus

i changed it to int . and had it return 0; still compiles w/out errors but will crash when run.

Ok, it has something to do with the way you are doing the strcpy, specifically how the pointers are being used. I added a line:

cout << "Had a,b,c,d: " << a << ", " << b << ", " << c << ", " << d << endl;
at the begining of the automobiles::set_values (before the assignment) and it would print
those values out with no problem and then core dump (I am using g++).

I did the following to your function (just assigned the pointer to a pointer):

void automobile::set_values (char *a, char* b, char* c, char* d)
{
cout << "Had a,b,c,d: " << a << ", " << b << ", " << c << ", " << d << endl;
// strcpy(make, a);
// strcpy(model, b);
// strcpy(color, c);
// strcpy(engine, d);
make = a;
model = b;
color = c;
engine = d;
}

Recompiled and got:
Had a,b,c,d: honda, accord, blue, v6
hondaaccordbluev6

(no core dump).

Way to rusty to remember the ins and outs of pointers to be of much help (just kind of instinctively think about them now), but therein lies your problem...


/nebulus

Ok, 3 things:
1- you don't initialize your char pointers. So they point anywhere. Who knows what happens when you do that strcpy. You should either do a malloc or declare you char* as char arrays of fixed length (ex: char make[80];).

2- Using strcpy IS VERY DANGEROUS IF YOU DON'T PROPERLY CHECK BOUNDS like you are doing: it's one of the worst "buffer overflow" causes... You should either use something like strncpy( char*, const char *, size) which will copy maximum "size" chars into the buffer hence avoiding an overflow, or very rigourously check bounds or malloc your char buffers dynamically with the right length.

3- Since you're doing C++, why don't you use the String class (#include <string>) and avoid yourself all the pain and gore of dealing with char* and string manip fonctions?


Ammo

Thanks ammo for the brush up on char (and cleaning up my muddled explanation), I have been rather happy to leave that mess behind (at least with the string manipulation) with a move over to perl and the like :)

One question though:

You mention the danger of using strcpy (which I understand where you are coming from), but if you are just assigning a pointer to a predefined/preset string, which is what I did, do you still have an issue with a possible buffer overflow (maybe their is an internal limit to the amount of chars/size of array that is inherent when compiled? is that what you mean?). To continue the question, say you took input (which is where the danger can arise) and assigned it to a * char, once again would assigning the pointer to the pointer be a problem or would the S already have hit the F, on the input (so long as there were no further modifications to the array of chars, such as allowing reinput (which I can quite obviously see as an issue))...

Thanks in advance,

Nebulus

Just to make things clear, my previous anwser refered only to the original poster (lowtone)'s code, it wasn't refering to yours nebulus200...

However there would still indeed be some problems with the way you modified it nebulus:
while replacing the strcpy with directy pointer reassignment avoids buffer overflows, it's problematic in the object logic... By pointing to a string literal or other char array outside the class, you loose the encapsulation effect: if the (well not string literal since it obviously can't be changed) char array to which you point outside gets modified or freed or whatever, your object's state will also be modified... Not the usual wanted effect.

As for the second part of your question, the thing is you can't assign "input" to a char pointer like that... "Taking input" requires passing the address of a buffer where the input will be stored. That buffer space HAS to be initialised in one way or another, either statically (char buffer[10] ) or dynamically (char * buffer; buffer = (char*)(nbchars * sizeof(char)); ). Either way, the buffer will be of limited space and could possibly be overflowed (depending on the input fucntion). If you then were to have another pointer point to that buffer, no that second pointer assignment could not cause a buffer overflow, but as said previously, it wouldn't be a copy of the string but rather "become the string" by pointing at it.

Hope this clears it up (and not mix it up more!)

Ammo

Indeed, just have gotten rusty with C++ I guess. It has been so long since I have had to think about the intracacies of doing strings/char input in C++, guess I looked right past it. Now that I have been thinking about it, I guess I did usually define the char array to store strings (when I went that route and didn't use String.h) statically and then used pointers to point at it (at which case I would have a problem without sanity checking :) ).

Thanks for the enlightenment.

/nebulus

To preserve encapsulation you'll have to pass the size of each character string.

make = new char[size];
*make = *a;

And repeat for the others.

Thankyou very much . everyone