Troubleshooting Segmentation Fault Involving Pointer

[dotnabox]

Or at least I think that's the issue. Essentially, I am in the process of overloading operators and allowing users to enter data directly into the class objects. I was able to do the first one so I followed the same pattern and I get a segmentation fault when I try to cin >> the information. Everything else about the program operates as it should, so I'll post what I think are the relevant pieces of code:

appointment.h:

class Appointment
{
  Time startTime;
  Time endTime;
  char *subject;
  char *location;
  friend ostream &operator<<( ostream &, const Appointment & );
  friend istream &operator>>( istream &, Appointment & );
public:
  Appointment();
  Appointment(const char* subject);
  ~Appointment();
  void read();
  Appointment& operator= (const Appointment &a);
  bool operator== (const char *s) const;
  void makeAppt();
}; // class Appointment

appointment.cpp:

void Appointment::makeAppt()
{
    cout << "Subject >> ";
    cin >> inputAppt;
}

istream &operator>>( istream &input, Appointment &appt )
{
  input >> appt.subject;
  return input;
}

Appointment::Appointment() : subject(NULL), location(NULL)
{
}

Thanks for taking a look.

Edit: Added in Appointment constructor.

[boblied]

appt.subject is a char*. What does it point at? Abandon char* and use std::string.

[dotnabox]

appt.subject is a char*. What does it point at? Abandon char* and use std::string.

Honestly, pointers are my weak point. We use the professor's code from the previous assignment and build off of it. Next week we can use string, finally, so there will be less of this.

In terms of what is points to, I'm not entirely sure to be honest. I've been starting at my code for a while now and I'm confused at the char *subject and char *location in the Appointment class.

Its happening because subject is not pointing to a valid address, basically you haven't allocated memory to it. Either use char subject[size] or allocate memory in constructor using malloc() or new operator.

[dotnabox]

Its happening because subject is not pointing to a valid address, basically you haven't allocated memory to it. Either use char subject[size] or allocate memory in constructor using malloc() or new operator.

Using code similar to other references to subject in Appointment, I tried this:

istream &operator>>( istream &input, Appointment &appt )
{
  char *ptr;
  appt.subject = new char[strlen(ptr) + 1];

  input >> appt.subject;
  return input;
}

It still compiles and segfaults, although now earlier.

Edit: It does work, however, with appt.subject = new char[integer value]. I know he'll want dynamic allocation, though.

How would I dynamically allocate memory to it properly?

The problem is again sort of same.

appt.subject = new char[strlen(ptr) + 1];

. Neither ptr holds any string nor it point to any dynamically allocated memory and you are still trying to calculate its length using strlen!

We could only guess what will be size of user input(unless you take it from user in runtime) and according to that we give some memory to it prior taking input.

So you have two options:
1. Either guess

appt.subject = new char[60];

. 60 is usually more than enough for this variable I suppose. However unless you have reasons, statically allocating memory is preferable.

2. Or ask user using cin, the size of subject and then allocate that much......foolish approach but would satisfy your professor if he needs use of dynamic memory allocation ;)

[dotnabox]

Thanks. I think I'll just stick with setting it to 60, at least for now. Once I finish the project, I'll go back and take another serious look at it. So consider the initial problem solved, then. I have quickly run into another snag, though, if you have another moment to spare.

I need to read in both sobject and location from this, but I'm really not sure how to go about it. With that code, calling the >> operator will always store it into appt.subject, but what about storing information to appt.location? Any suggestions on an approach?

[boblied]

You've just moved the uninitialization. ptr is a random number.

ptr, subject, and location are just 4 tiny little bytes that are the address of some other memory. If you leave them uninitialized or set to NULL, then they're pointing at nothing, or at least nothing useful. They have to become the address of something, or the program will blow up when trying to use them.

This is the basic problem with char*. You have to guess what the maximum size might be and allocate enough space. If you guess wrong, then hackers overflow your buffers and steal your bank account. If you don't guess at all, your program crashes.

To solve this problem, in the constructor of Appointment, allocate space for subject and location, and recover it in the destructor.

const size_t BiggestLikelyString = 2048;
Appointment::Appointment()
{
    subject = new char[BiggestLikelyString];
    location = new char[BiggestLikelyString];
    subject[0] = location[0] = '\0';
}
Appointment::~Appointment()
{
    delete[] subject;
    delete[] location;
}

[dotnabox]

@boblied: I do like that a lot better than what I currently have. I think when I go through and revise it after I get it to all function properly I'll switch over to that. Like I said, though, this will become less of an issue once we are allowed to use strings next week.

[boblied]

I need to read in both sobject and location from this, but I'm really not sure how to go about it. With that code, calling the >> operator will always store it into appt.subject, but what about storing information to appt.location? Any suggestions on an approach?

Each time you use operator>>() , it will read one word. For the purposes of the exercise, you could assume that subject will be one word and location will be another word. If either one could be a line of multiple words, you will have to use the getline() function (see standard library documentation).

istream& operator>>(istream& is, Appointment& a)
{
    cout << "Enter subject and location:" << endl;
    is >> a.subject >> a.location;
    return is;
}

or

istream& operator>>(istream& is, Appointment& a)
{
    cout << "Enter subject, end with carriage return:" << endl;
    is.getline(a.subject, BiggestLikelyString);
    cout << "Enter location, end with carriage return:" << endl;
    is.getline(a.location, BiggestLikelyString);
}

To avoid buffer overflow, prefer using cin.getline(char_ptr, size); >> With that code, calling the >> operator will always store it into appt.subject, but what about storing information to appt.location? Any suggestions on an approach?
Actually there are more than one ways to do that. Simplest being :

in >> subject >> location

However you get few problems:
1. Cant use space, in strings
2. Buffer overflow
3. Will surely confuse the user

So you print message before every input like you do in normal program:

cout << "\nEnter Location: ";
in.getline(location, 60);
cout << "\nEnter Subject: "
in.getline(subject, 60);

[dotnabox]

The second one is exactly what I'm looking for. I don't know how I forgot about using getline. My problem is, how would I call that operator in the first place? I feel like the order would be off.

I very specifically have to have my Day class call this Appointment class, which is how it is now, but then wouldn't I need some sort of cout statement before I call the >> operator? In which case, wouldn't I then have cout statements in both my makeAppt() function and also in my overloaded operator? It seems messy to me. Or I'm missing something.

[dotnabox]

So I think part of my problem is that I haven't fully explained exactly what I am looking for. Both of your responses are spot on, I understand them, but they don't quite fit my need here and I'm not sure how to adapt them. Let me try to clarify.

The exact console should look like this:

Calendar Menu
0. Done.
1. Search for date.
2. Search for subject.
3. Add an appointment.
Your choice >> 3

Please enter the month and day (mm/dd) >> 10/29
Subject >> Appointment
Location >> 3052 Kemper
Start time >> 6:15 PM
End time >> 7:37 PM

The overloaded >> operator in the Day class will then call the overloaded >> operator or the Appointment class (what we've been talking about this whole time), which will finally call the overloaded >> operator in the Time class.

Therefore, I need to use the overloaded operator in my Appointment class to read in an Appointment and store the user input to *location and *subject, but they must be on separate lines, as shown above.

And thank you two for all your help. I'm writing down all of what you've told me as it's invaluable, even beyond thescope of my current assignment.

[boblied]

I very specifically have to have my Day class call this Appointment class, which is how it is now, but then wouldn't I need some sort of cout statement before I call the >> operator? In which case, wouldn't I then have cout statements in both my makeAppt() function and also in my overloaded operator? It seems messy to me. Or I'm missing something.

Glad it's been helpful; one more guess before I call it a day. You could delegate makeAppt to operator>> and leave the appointment prompting in operator>> .

void Appointment::makeAppt()
{
    cin >> *this;
}

[dotnabox]

That will do the trick. I know it's not what he had in mind, but again, I can revise it later. Thanks for everything!