3

Ok, this is for homework about hashtables, but this is the simple stuff I thought I was able to do from earlier classes, and I'm tearing my hair out. The professor is not being responsive enough, so I thought I'd try here.

We have a hashtable of stock objects.The stock objects are created like so:

stock("IBM", "International Business Machines", 2573, date(date::MAY, 23, 1967))

my constructor looks like:

stock::stock(char const * const symbol, char const * const name, int sharePrice, date priceDate): m_symbol(NULL), m_name(NULL), sharePrice(sharePrice), dateOfPrice(priceDate)
{    
setSymbol(symbol);
setName(name);
}

and setSymbol looks like this: (setName is indentical):

void stock::setSymbol(const char* symbol)  
{  
if (m_symbol)  
    delete [] m_symbol;  
m_symbol = new char[strlen(symbol)+1];  
strcpy(m_symbol,symbol);  
}

and it refuses to allocate on the line

m_symbol = new char[strlen(symbol)+1];

with a std::bad_alloc. name and symbol are declared

char * m_name;  
char * m_symbol;

It's definitely strlen() that is going astray. And it doesn't seem to happen every time. 

cout << symbol << strlen(symbol);

returns IBM correctly, then crashes

quandrum
  • 1,608
  • 1
  • 11
  • 14
  • name and symbol are class variables, as well as the function parameters in the constructor and the setSymbol method. Maybe a bit confusing. – quandrum May 10 '10 at 17:02
  • 1
    Do a `std::cout << strlen(symbol)` before the line that fails. Check that the string length value makes sense. – Emile Cormier May 10 '10 at 17:04
  • 1
    You should adopt a naming convention for your member variables. Two popular ones are `memberVariable_` and `m_memberVariable`. Such a convention will make it easier to distinguish parameter names and member variable names. – Emile Cormier May 10 '10 at 17:06
  • ok, so changing the variable names worked. I think there was too much confusion between variable names going on. – quandrum May 10 '10 at 17:17
  • This question made me spawn a new, more general question here: http://stackoverflow.com/questions/2804880 – Emile Cormier May 10 '10 at 17:31
  • Ack! I changed my variable names as I mentioned. My code ran fine 4 times, and now the problem is back. It's definitely the strlen(symbol) that's borking. std::cout << symbol << strlen(symbol) prints out the expected IBM, then gives me a std::bad_alloc on the strlen(symbol). This seems so silly. My hashtable implemenation works beautifully though. – quandrum May 10 '10 at 17:56
  • 1
    Some pointer manipulation error might be causing the terminating `'\0\'` after "IBM" to get overwritten, resulting in a bogus strlen value. – Emile Cormier May 10 '10 at 18:04
  • is there anyway to tell this? I haven't manipulated the string at this point, just passed it straight through the constructor. – quandrum May 10 '10 at 18:11
  • When a single-threaded program crashes only some of the time, it's usually because something is not initialized properly before use (it is initialized to random garbage that happens to be in memory). – Emile Cormier May 10 '10 at 18:11
  • Can you post (in the same question, use the edit function) a minimal version of your program that compiles and causes the error? I and others can take a look and help you pin-point where the problem is. If it's too large to post here, post it at some code snippet website and post the link. (But you're more likely to get help if you keep it short.) – Emile Cormier May 10 '10 at 18:13
  • 1
    Are you defining your own copy constructor and assignment operator? If you don't, the default ones generated by the compiler will perform shallow copies of your member variables (this is not what you want). See http://en.wikipedia.org/wiki/Object_copy, http://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy. – Emile Cormier May 10 '10 at 22:19

4 Answers4

1

As this is tagged C++ can you use std::string instead of doing all the pointer maintenance yourself on char*?

std::string name;
std::string symbol

Then setSymbol becomes easy:

void stock::setSymbol(const char* symbol)  
{
    this->symbol = symbol;
}
Mark B
  • 95,107
  • 10
  • 109
  • 188
  • 4
    I do so hate it when C++ teachers demand that students use the C way of doing things, for no reason other than that the teacher is a C programmer wearing a hat that says `++`. It is certainly valuable to know how strings work, but when it impedes understanding of other concepts, there is no sanity in demanding it. – Jon Purdy May 10 '10 at 17:05
  • 2
    @Quandrum: It might be easier to do the conversion just to see if it fixes the current bug, and then switch back. – Potatoswatter May 10 '10 at 18:53
1

There must be some problem with symbol parameter at the time you call

new char[strlen(symbol)+1];

and strlen return a huge length that C++ runtime is unable to allocate. If symbol is uninitialized char* pointer at the beginning this is fairly possible. It doesn't fail all the time, does it?

pajton
  • 15,828
  • 8
  • 54
  • 65
  • I guess having the variables the same name is confusing. The symbol in strlen(symbol) is the function parameter, which points to the string "IBM" at this point. this->symbol is a class variable, which is a null pointer at this point. However, it doesn't fail all the time. You are correct. – quandrum May 10 '10 at 17:00
  • A silly question (I would test If I had a compiler at hand but right now I don't) : is it possibel that the 'symbol' parameter and the 'symbol' instance variable somehow gets mixed up ? So at runtime strlen would run on a non-initialized string and give a bogus length ... Intuitively, you would think the compiler canno't be fooled, but with C++, you never know ;) – phtrivier May 10 '10 at 17:01
  • @phtrivier: Not a silly question. This problem is called variable shadowing: http://en.wikipedia.org/wiki/Variable_shadowing – Emile Cormier May 10 '10 at 17:13
  • @emile : thanks for the link... as you pointed elsewhere, that's the kind of errors you transparently get rid of by using different naming conventions for member / static / local variables ... but then *choosing* a convention opens the door to endless trolls ;) – phtrivier May 11 '10 at 07:52
0

I was able to run the code without problems on Cygwin, so I'd guess it's something implementation-dependent in distinguishing the parameter symbol from the member symbol.

You say yourself that it's confusing -- well do something about it!!! And may I suggest, never, ever again, naming a parameter the same as a local/member variable. (Not only does it eliminate confusion, you won't need to disambiguate the member variable with this->.)

Paul Richter
  • 6,154
  • 2
  • 20
  • 22
  • Mine runs without problems sometimes... even after I changed variable names to make it none confusing. – quandrum May 10 '10 at 18:02
0

Thanks to everyone who offered help. I went over it with my professor, and unfortunately I was overflowing an array earlier and corrupting the heap, which was manifesting itself here.

This was a good conversation for me though. It helped me think through some things I had just been doing. So thanks again SO'ers

quandrum
  • 1,608
  • 1
  • 11
  • 14