Runtime error: std::bad_alloc at memory location

[schizoid_man]

Hi,

I have the following code snippets and I get a std::bad_alloc error
where I think there should be none. I've attached the relevant bits of
the base class, derived class and the .cpp file containing the main()
method.

I'd really appreciate any help in getting this error sorted out.

Thanks,
Anuj
Base class:

onst int DESCRIPTION_LENGTH = 128;

class InventoryItem {

private:
char* itemDescription;
int itemQuantity;
double itemCost;

void assignDescription(const char* desc) {
int stringLength = strlen(desc);
itemDescription = new char[stringLength + 1];
strncpy(itemDescription, desc, DESCRIPTION_LENGTH);
}

public:
// Constructor
InventoryItem(const char* desc, int qty, double cost) {
assignDescription(desc);
itemQuantity = qty;
itemCost = cost;
}

// Destructor
~InventoryItem(void) {
delete [] itemDescription;
}
}

Derived class:

const int DEFAULT_SIZE = 20;

class InventoryList {

private:
InventoryItem **inventory;
int count;

public:
// Constructor
InventoryList(void) {
inventory = new InventoryItem*[DEFAULT_SIZE];
count = 0;
}
// Destructor
~InventoryList(void) {
for (int i = 0; i < count; i++)
delete inventory[i];
delete [] inventory;
}

// Method to add InventoryItem object to InventoryList object
void addItem(InventoryItem* item) {
inventory[count++] = item;
}
}

Main .cpp:

#include "InventoryItem.h"
#include "InventoryList.h"
#include <iostream>

using namespace std;

int main(void)
{
InventoryList* inventory = new InventoryList[10];

inventory->addItem(new InventoryItem("wrench", 3, 21.99));
inventory->addItem(new InventoryItem("hammer", 9, 1.99));
inventory->addItem(new InventoryItem("pliers", 2, 2.99));
inventory->addItem(new InventoryItem("saw", 6, 3.99));

inventory->printList();

return 0;
}

[Andrey Tarasevich]

schizoid_man wrote:

...
I have the following code snippets and I get a std::bad_alloc error
where I think there should be none. I've attached the relevant bits of
the base class, derived class and the .cpp file containing the main()
method.

One can find many obvious and potential problems with your code if one
assumes that this is a _complete_ code. I will assume that this is just
a fragment, so I'll consider only the problems with the code that is
explicitly present here.

Also, I don't see any "base" and "derived" classes in your code. You
have two unrelated (from the language point of view) classes. Anyway...

>
Base class:

It is not really a "base" for anything in the quoted code...

onst int DESCRIPTION_LENGTH = 128;

const int?

>
class InventoryItem {

private:
char* itemDescription;
int itemQuantity;
double itemCost;

void assignDescription(const char* desc) {
int stringLength = strlen(desc);
itemDescription = new char[stringLength + 1];
strncpy(itemDescription, desc, DESCRIPTION_LENGTH);

Here you allocate 'stringLength + 1' bytes of memory, but then you copy
up to 'DESCRIPTION_LENGTH' bytes into the allocated block. Remember,
this 'strncpy' will _always_ overwrite 'DESCRIPTION_LENGTH' bytes in the
target memory block, even it the source string is shorter than that. In
case when 'desc' is shorter, the 'strncpy' won't stop, but rather it
will fill the remaining bytes of the destination block with zeros.

In other words, here you have a clear case of memory overrun - a very
real reason of undefined behavior, which can easily lead to the
'bad_alloc' exception. 'strncpy' is not what you need in this case. In
this case you can safely use 'strcpy' instead. (Frankly, I don't
understand why you even need that 'DESCRIPTION_LENGTH' in your code).

Better yet, get rid of manual memory management and learn to use
'std::string' instead.

}

public:
// Constructor
InventoryItem(const char* desc, int qty, double cost) {
assignDescription(desc);
itemQuantity = qty;
itemCost = cost;
}

// Destructor
~InventoryItem(void) {
delete [] itemDescription;
}
}

Derived class:

It is not really "derived"...

const int DEFAULT_SIZE = 20;

class InventoryList {

private:
InventoryItem **inventory;
int count;

public:
// Constructor
InventoryList(void) {

'void' is not required here (as well as in other functions)... But
that's a matter of personal preference.

inventory = new InventoryItem*[DEFAULT_SIZE];
count = 0;
}
// Destructor
~InventoryList(void) {
for (int i = 0; i < count; i++)
delete inventory[i];
delete [] inventory;
}

// Method to add InventoryItem object to InventoryList object
void addItem(InventoryItem* item) {
inventory[count++] = item;

Since you don't grow your memory, it makes sense to check for the array
size here. Make sure you don't go over 'DEFAULT_SIZE' elements.

Or, better yet, learn to use 'std::vector<>' instead of allocating
arrays with 'new[]'.

}
}

Main .cpp:

#include "InventoryItem.h"
#include "InventoryList.h"
#include <iostream>

using namespace std;

int main(void)
{
InventoryList* inventory = new InventoryList[10];

Hm... You allocate an array of 10 'InventoryList's but use only the
first element later. What was the point of allocating 10 of them?
Although this is not a reason for the exception...

inventory->addItem(new InventoryItem("wrench", 3, 21.99));
inventory->addItem(new InventoryItem("hammer", 9, 1.99));
inventory->addItem(new InventoryItem("pliers", 2, 2.99));
inventory->addItem(new InventoryItem("saw", 6, 3.99));

inventory->printList();

return 0;
}

Aside from missing 'delete[]' (and no implementation for 'printList')
this 'main' looks fine. The most likely reason for the exception is that
'strncpy' in 'assignDescription'.

--
Best regards,
Andrey Tarasevich

[Gianni Mariani]

schizoid_man wrote:

Hi,

I have the following code snippets and I get a std::bad_alloc error
where I think there should be none. I've attached the relevant bits of
the base class, derived class and the .cpp file containing the main()
method.

I'd really appreciate any help in getting this error sorted out.

Thanks,
Anuj
Base class:

onst int DESCRIPTION_LENGTH = 128;

I assume u mean "c"onst

>
class InventoryItem {

private:
char* itemDescription;
int itemQuantity;
double itemCost;

void assignDescription(const char* desc) {
int stringLength = strlen(desc);
itemDescription = new char[stringLength + 1];
strncpy(itemDescription, desc, DESCRIPTION_LENGTH);

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^
This is your error. You're copying more bytes than allocated.

}

public:
// Constructor
InventoryItem(const char* desc, int qty, double cost) {
assignDescription(desc);
itemQuantity = qty;
itemCost = cost;
}

// Destructor
~InventoryItem(void) {
delete [] itemDescription;
}
}

// needs a ";"

>
Derived class:

const int DEFAULT_SIZE = 20;

class InventoryList {

private:
InventoryItem **inventory;
int count;

public:
// Constructor
InventoryList(void) {
inventory = new InventoryItem*[DEFAULT_SIZE];
count = 0;
}
// Destructor
~InventoryList(void) {
for (int i = 0; i < count; i++)
delete inventory[i];
delete [] inventory;
}

// Method to add InventoryItem object to InventoryList object
void addItem(InventoryItem* item) {
inventory[count++] = item;
}
}

needs another ";"

>
Main .cpp:

#include "InventoryItem.h"
#include "InventoryList.h"
#include <iostream>

using namespace std;

int main(void)
{
InventoryList* inventory = new InventoryList[10];

inventory->addItem(new InventoryItem("wrench", 3, 21.99));
inventory->addItem(new InventoryItem("hammer", 9, 1.99));
inventory->addItem(new InventoryItem("pliers", 2, 2.99));
inventory->addItem(new InventoryItem("saw", 6, 3.99));

inventory->printList();

// function does not exist

>
return 0;
}

Give up on cstrings already... Use std::string and std::vector.

e.g.
#include <string>
#include <vector>

class InventoryItem {

private:
std::string itemDescription;
int itemQuantity;
double itemCost;

void assignDescription(const char * desc)
{
itemDescription = desc;
}

public:
// Constructor
InventoryItem(const char* desc, int qty, double cost)
: itemDescription( desc ),
itemQuantity( qty ),
itemCost( cost )
{
}
};

//Derived class:

const int DEFAULT_SIZE = 20;

class InventoryList {

private:
std::vector< InventoryItem inventory;
int count;

public:
// Constructor
InventoryList(void)
{
}

// Destructor
~InventoryList(void) {
}

// Method to add InventoryItem object to InventoryList object
void addItem(const InventoryItem& item)
{
inventory.push_back( item );
}
};

//Main .cpp:

//#include "InventoryItem.h"
//#include "InventoryList.h"
#include <iostream>

using namespace std;

int main(void)
{
InventoryList* inventory = new InventoryList[10];

inventory->addItem(InventoryItem("wrench", 3, 21.99));
inventory->addItem(InventoryItem("hammer", 9, 1.99));
inventory->addItem(InventoryItem("pliers", 2, 2.99));
inventory->addItem(InventoryItem("saw", 6, 3.99));

// inventory->printList();

return 0;
}

[Schizoid Man]

Gianni Mariani wrote:

> void assignDescription(const char* desc) {
int stringLength = strlen(desc);
itemDescription = new char[stringLength + 1];
strncpy(itemDescription, desc, DESCRIPTION_LENGTH);

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^

Thank you both Gianni and Andrey. Replacing strncpy with strcpy did the
trick. As you'd noted, the code was not complete, but I thought I'd just
attach the relevant bits.

Thanks for the tip on <stringand <vector>. I'm going to take a look at
using both.