Buffer too small during object construction. Think it has something to do with strcpy_s()

Question

The code that I am putting up is the result of the third week of working on this class. I had a pretty good handle on things, (or so I thought), but this week is focusing on pointers and I am clueless as to why I keep getting this error. I keep getting a Debug Assertion Failed! error along with a general "Buffer is too small" explanation.

my error message

Here is the complete code that I have compiling on a Win8 OS using VS 2012 RC Version 11.0.505221.1. The only difference from what I have compiled in Linux is that I am using strcpy_s() in this code because for some reason MS doesn't like strcpy().

#include "stdafx.h"
#include <iostream>
#include <string>
#include <iomanip>
#include <limits>

using namespace std;

class HotelRoom
{
    char roomNumber[4];
    char guest[81];
    int roomCapacity, currentOccupants;
    double roomRate;

public:
    HotelRoom(char[], char[], int, double);
    ~HotelRoom();
    void DisplayRoom();
    void DisplayNumber();
    void DisplayName();
    int GetCapacity();
    int GetStatus();
    double GetRate();
    void ChangeStatus(int);
    void ChangeRate(double);
};

HotelRoom::~HotelRoom() {
cout << endl << endl;
cout << "Room #" << roomNumber << " no longer exists." << endl;
delete [] guest;
}

void HotelRoom::DisplayName() {
cout << guest;
}

void HotelRoom::DisplayNumber() {
cout << roomNumber;
}

int HotelRoom::GetCapacity() {
return roomCapacity;
}

int HotelRoom::GetStatus() {
return currentOccupants;
}

double HotelRoom::GetRate() {
return roomRate;
}

void HotelRoom::ChangeStatus(int occupants) {
if(occupants <= roomCapacity) {
    currentOccupants = occupants;
}
else {
    cout << endl << "There are too many people for this room. Setting occupancy to -1." << endl;
    currentOccupants = -1;
}
}

void HotelRoom::ChangeRate(double rate) {
roomRate = rate;
}

HotelRoom::HotelRoom(char room[], char guestName[], int capacity, double rate)
{
strcpy_s(roomNumber, room);     //Compiles fine with strcpy on Linux, but MS is making me use strcpy_s to compile
guestName = new char[strlen(guestName) + 1];
strcpy_s(guest, guestName);     //Same as above
roomCapacity     =  capacity;
currentOccupants = 0;
roomRate         = rate;
}

void HotelRoom::DisplayRoom()
{
cout << setprecision(2)
     << setiosflags(ios::fixed)
     << setiosflags(ios::showpoint);
cout << endl << "The following is pertinent data relating to the room:\n"
     << "Guest Name:        " << guest << endl
     << "Room Number:       " << roomNumber << endl
     << "Room Capacity:     " << GetCapacity() << endl
     << "Current Occupants: " << GetStatus() << endl
     << "Room Rate:         $" << GetRate() << endl;
}


int main()
{
int numOfGuests;
char roomNum[4]; 
char buffer[81];    //Buffer to store guest's name
int roomCap;
double roomRt;
bool badInput = true;
cout << endl << "Please enter the 3-digit room number: ";
do {        //loop to check user input
    badInput = false;   
    for(int x = 0; x < 3; x++)
    {
        cin >> roomNum[x];
        if(!isdigit(roomNum[x]))        //check all chars entered are digits
        {
            badInput = true;
        }
    }
    char x = cin.get();
    if(x != '\n')       //check that only 3 chars were entered
    {
        badInput = true;
    }
    if(badInput)
    {
        cout << endl << "You did not enter a valid room number. Please try again: ";
    }
} while(badInput);
for(;;)     //Infinite loop broken when correct input obtained
{
    cout << "Please enter the room capacity: ";
    if(cin >> roomCap) {
        break;
    } else {
        cout << "Please enter a valid integer" << endl;
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    }
}
for(;;)     //Infinite loop broken when correct input obtained
{
    cout << "Please enter the nightly room rate: ";
    if(cin >> roomRt) {
        break;
    } else {
        cout << "Please enter a valid rate" << endl;
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    }
}
cin.get();      //Dump the trailing return character
cout << "Please enter guest name: ";
cin.getline(buffer, 81);
HotelRoom room1(roomNum, buffer, roomCap, roomRt);
for (;;) {      //Infinite loop broken when correct input obtained
cout << "Please enter the number of guests for room #";
room1.DisplayNumber();
cout << ": ";
    if (cin >> numOfGuests) {
        break;
    } else {
        cout << "Please enter a valid integer" << endl;
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    }
}
room1.ChangeStatus(numOfGuests);
room1.DisplayRoom();
cout << endl << "The following shows after the guests have checked out." << endl;
room1.ChangeStatus(0);
room1.DisplayRoom();
room1.ChangeRate(175.0);
for (;;) {      //Infinite loop broken when correct input obtained
cout << "Please enter the number of guests for room #";
room1.DisplayNumber();
cout << ": ";
    if (cin >> numOfGuests) {
        break;
    } else {
        cout << "Please enter a valid integer" << endl;
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    }
}
room1.ChangeStatus(numOfGuests);
room1.DisplayRoom();
return 0;
}

UPDATE:

I've added cout statements to see where the problem occurs in the program, and it is definitely at the strcpy() statements in the HotelRoom constructor. Here is the constructor, and following is the output that I received

HotelRoom::HotelRoom(char room[], char guestName[], int capacity, double rate)
{
cout << endl << "Attempting 1st strcpy...";
strcpy_s(roomNumber, room);     //Compiles fine with strcpy on Linux, but MS is making me use strcpy_s to compile
cout << endl << "1st strcpy successful!";
guestName = new char[strlen(guestName) + 1];
cout << endl << "Attempting 2nd strcpy...";
strcpy_s(guest, guestName);     //Same as above
cout << endl << "2nd strcpy successful!";
roomCapacity     =  capacity;
currentOccupants = 0;
roomRate         = rate;
}

cout output

You should start acquainting yourself with the concept of "narrowing the problem down", and "telling others only what they need to know". Other useful things are making your own small tests to see if each part works the way you expect. — Kerrek SB, Nov 13 '12 at 04:13
You should use strcpy and ignore MS. They're not really doing anyone any favours by warning about perfectly valid C++. If you want that level of safety, you should be programming in BASIC :-) — paxdiablo, Nov 13 '12 at 04:13
@paxdiablo while I love throwing hot-irons at MS as much as the next guy, strcpy_s() in this case did exactly what it was supposed to: flag a big-arse assertion in your face that you just overran a buffer. Release mode is worse, btw. It just terminates the program outright. — WhozCraig, Nov 13 '12 at 05:00
How does this work when the [Microsoft manual](http://msdn.microsoft.com/en-us/library/td1esda9%28v=vs.80%29.aspx) says the prototype for `strcpy_s()` is: `errno_t strcpy_s(char *strDestination, size_t numberOfElements, const char *strSource);`? Is there a separate meaning for `strcpy_s()` in C++ defined on some other page? (ISO/IEC 9899:2011, Annex K.3.7.1.3, previously TR 24731-1, gives the signature: `errno_t strcpy_s(char * restrict s1, rsize_t s1max, const char * restrict s2);`, which is close enough the same. — Jonathan Leffler, Nov 13 '12 at 06:42

score 1 · Accepted Answer · answered Nov 13 '12 at 04:53

I think you may need to look at this again:

guestName = new char[strlen(guestName) + 1];
cout << endl << "Attempting 2nd strcpy...";
strcpy_s(guest, guestName);     //Same as above

I'm fairly sure, since guestName[] is a parameter, you intent was NOT to lose that pointer for good in the function scope, replacing it with a freshly allocated, non-terminated pointer, then proceeding to copy uninitialized memory to your member variable.

Perhaps you wanted this instead:

strcpy_s(guest, guestName);

Also, guest is a member variable of type char[81]. unless you want the heap manager to throw that nasty dialog up again, you may want to avoid doing this in the class destructor:

delete [] guest;

which is deleting non-heap memory and all-but-guaranteed to make the heap manager puke.

score 0 · Answer 2 · answered Nov 13 '12 at 04:17

0

You defined char buffer[81], but you are trying to read 81 characters cin.getline(buffer, 81) excluding the \0. So you need to change the earlier one to char buffer[82] or later one to cin.getline(buffer, 80).

And if you are using C++ why not using string?

answered Nov 13 '12 at 04:17

xiaoyi

6,641
1
34
51

It's fairly common in colleges using c++ to not allow you to use the string class or std libraries. – evanmcdonnal Nov 13 '12 at 04:23

score 0 · Answer 3 · answered Nov 13 '12 at 04:22

0

The reason why MS is asking you to use _s function is to protect your code against buffer overflows like your code currently displays. Check this link for more information.

You have a string of 81 characters. You new a new string of its size + 1 (you will lose your data) which is now 82 characters. When you try to copy your buffer into guest which is 81 characters long, you get an assert.

Consider using std::string and stop allocating / freeing strings, it's of no use in your case.

answered Nov 13 '12 at 04:22

emartel

7,712
1
30
58

That is the point for this project. I am supposed to use the pointers. Believe me, I would much rather use the string class, but that isn't an option right now. – Nyxm Nov 13 '12 at 04:25
As xiaoyi pointed out, change your get line to fit within your buffer, then remove the new / delete as they serve no purpose (you already have a static array to hold your guest) – emartel Nov 13 '12 at 04:28

score 0 · Answer 4 · answered Nov 13 '12 at 04:27

0

If you don't use strcpy_s you need to do bounds checking yourself before copying from a pointer into a buffer.

Also you shouldn't call delete on guest. Delete is used to free memory you allocated using new.

answered Nov 13 '12 at 04:27

evanmcdonnal

46,131
16
104
115

Buffer too small during object construction. Think it has something to do with strcpy_s()

4 Answers4