Logical Error

[Mathhead200]

Bump.

[Mathhead200]

This error is really starting to... bug... me. (That was bad.)
But really, if anyone can please throw me some ideas I would be more then willing to entertain them.
Please!

[firedraco]

I don't know...try stepping through it with a debugger maybe. I've never had that kind of error happen before.

[Mathhead200]

Can you suggest one (a debugger?) I have Visual Studios 2008 on my computer, but I only use it's command line compiler (cl.exe). I don't really like it as an IDE. (I use notepad++ because it's just a syntax highlighter, less overhead.)

[firedraco]

VC++ has a great debugger (and IDE) IMO. Just set a breakpoint (click the left column next to the line you want to stop at), then run it. You can then use the Debug menu to step through the code and figure out exactly where it breaks.

[b2ee]

I compiled the codes with g++ somehow ( I don't have a VC debugger either). Here is some clue which may be helpful for you. It looks the problem is not from the copy constructor, but from when descontructor of bank(8642) is called when the main exists.

I'll take a more look once I get more spare time.

Account E: #86420004
Account F: #86420005

Breakpoint 1, main (argc=1, argv=0x3e26a8) at banker.cpp:76
76              Bank bankCopy = bank;
(gdb) n
78              return 0;
(gdb) s
76              Bank bankCopy = bank;
(gdb) s
bank::Bank::~Bank (this=0x22f7e4, __in_chrg=<value optimized out>) at bank/Bank.h:67
67              Array<Integer> indexes = accounts.keys();
(gdb) continue
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x7c96f749 in ntdll!RtlpNtMakeTemporaryKey () from C:\WINDOWS\system32\ntdll.dll
(gdb) bt
#0  0x7c96f749 in ntdll!RtlpNtMakeTemporaryKey () from C:\WINDOWS\system32\ntdll.dll
#1  0x7c94bc4c in ntdll!LdrFindEntryForAddress () from C:\WINDOWS\system32\ntdll.dll
#2  0x003e0000 in ?? ()
#3  0x7c927573 in ntdll!RtlPcToFileHeader () from C:\WINDOWS\system32\ntdll.dll
#4  0x77c2c2de in msvcrt!free () from C:\WINDOWS\system32\msvcrt.dll
#5  0x003e0000 in ?? ()
#6  0x0040d259 in mathhead200::String::~String (this=0x3e27b0,
    __in_chrg=<value optimized out>) at mathhead200/String.h:44
#7  0x0040d859 in bank::Account::~Account (this=0x3e2790, __in_chrg=<value optimized out>)
    at bank/Account.h:11
#8  0x00404828 in bank::Bank::~Bank (this=0x22f80c, __in_chrg=<value optimized out>)
    at bank/Bank.h:69
#9  0x004063fc in main (argc=1, argv=0x3e26a8) at banker.cpp:62
(gdb)

[Mathhead200]

Yea, the VC++ debugger says it's crashing because of a bad pointer in the mathhead200::String destructor. I'm looking at it now too.

[firedraco]

Sounds like you are deleting the pointer twice. Make sure when you copy pointers you do a deep copy and not a shallow one.

[Mathhead200]

firedraco

Sounds like you are deleting the pointer twice.

I was thinking the same thing. I'm still working on it, and will be back as soon as a figure anything out.

[Mathhead200]

Here is what I think is happening...

File: "Major Project/src/bank/Bank.h"

Bank::~Bank() {
	Array<Integer> indexes = accounts.keys();
	for( int i = 0; i < indexes.getSize(); i++ )
		delete accounts[indexes[i]]; //here we call the Account destructor
		//	the (default) Account destructor calls the String destructor (see below)
	Array<Array<unsigned int>*> ids = names.values();
	for( int i = 0; i < ids.getSize(); i++ )
		delete ids[i];
}

File: "Major Project/src/mathhead200/String.h"

class String : public HashElement
{
	char *value;
	...
 public:
	~String() { delete value; } //here we are deleting a bad pointer
	...
};

Why? I can't figure out why this is happening. I hope this points someone in the right direction. I'll keep checking, thanks for the help so far, and let me know if anyone has anymore ideas or figures anything out.

[b2ee]

Take another quick look. I can't see where the account name is deeply and fully copied. One way to debug this is: print all addresses in "String" which is assigned and deleted in the whole cycle ( add some printing of the addresses in when "new" and "delete"). I usually don't use "delete" and "new" directly because it's hard to trace the memeory assignment. Usually I woudl wrap "delete" and "new" in some macro to trace the mememory.

I suspect there is memeory leaking and confilict use in "String". There are a lot of "new", but seldome "delete". For example, when the address to "value" is "delete", it's better to assign it back to "NULL", so before next "new", at least you know the "value" is usable.

Anyway, add more printing when "delete" and "new", so you can monitor their activities. I Will have another look when free.

Good luck.

b2ee

[Mathhead200]

I still haven't figured this bug out yet.

[Mathhead200]

Bump.

[b2ee]

Couldn't get much time to look at it. Some clue below ( I changed the copy constructor a little to add identifier in and 6 accounts).

Below printing is out when deconstructing is happening, obviously, some of account information has been copied, but it looks not what you want. ( Do you need copy accounts to cross the banks?)

And you can see the "String" addresses use same memory in two banks.

By the way, the "String" class is a little strange. I couldn't see why it was implemented as that.

Deconstructor start
The bank number: 8643 deconstruct started
String Value is deletedAccount A: 003e26f4
String Value is deleted: 003e26dc
String Value is deletedAccount B: 003e6c04
String Value is deleted: 003e6bec
String Value is deletedAccount C: 003e6d1c
String Value is deleted: 003e6d04
String Value is deletedAccount D: 003e6dac
String Value is deleted: 003e6d94
String Value is deletedAccount E: 003e6e44
String Value is deleted: 003e6e2c
String Value is deletedAccount F: 003e268c
String Value is deleted: 003e2674
String Value is deleted: 0022f63c
String Value is deleted: 0022f648
String Value is deleted: 0022f6ac
String Value is deleted: 003e2ad0
The bank number: 8642 deconstruct started
String Value is deletedΦn>: 003e26f4
String Value is deleted╨k>: 003e26dc
String Value is deleted0m>: 003e6c04
String Value is deleted('>: 003e6bec
String Value is deleted╪m>: 003e6d1c
String Value is deletedxm>: 003e6d04
String Value is deleted 

[coder777]

Your zip is way too much to review. But following will crash:

	Array<T>& operator=(Array<T> arr) { // This parameter will make a shallow of 'array' and delets it when the 'operator=' is done
		size = arr.size;
		length = arr.length;
		delete[] array;
		array = new T[length];
		copy(arr.array, array, size);
		return *this;
	}

It must be: Array<T>& operator=(const Array<T> &arr) { // No copy involved

Look if you have more operators like that. const X &x is always the safer option!

[kbw]

Ok, I've run your original code, I don't know if it's been updated since you first posted. You're right, it crashes when loading from file.

Before I get into it, have you never heard of the standard library? There's the C++ standard library that has things like vector and string that you've reproduced and there's the C standard library that has things like memcpy that you've reproduced. Use of the standard library shows a sophistication in the user; plus, its been tested and it works.

Have you stepped thru each line of your code in a debugger and seen it do what you think it should be doing? If not, you should.

If you did, you'd notice that you're passing your strings by value, thus littering your app with temporary strings. Your string class allocates blocks of memory with vector new, but release with scalar delete.

Why does Bank::add(Account*) modify the account number?

What is the difference between:

Bank& Bank::operator=(Bank bank)

and

Bank& Bank::operator=(const Bank &bank)

Once you've answered that, decide what's the effect of misakenly defining the former instead of the latter.

You (mis)used that construct througout your app. Fix it.

[Mathhead200]

Thanks b2ee, I'll take a look. I think that might just be where the problem is. I need to figure out why that's happening though so I can fix it... :)

coder777

This parameter will make a shallow of 'array' and delets it when the 'operator=' is done

Thanks for taking a look. That will actually call the copy constructor, which makes a deep copy. I think I'm ok there.

kbw

Before I get into it, have you never heard of the standard library?

http://cplusplus.com/forum/general/40262/#msg217586
See point 4, and my response.

kbw

Why does Bank::add(Account*) modify the account number?

Because adding the account to a bank causes it to take ownership of that account, not only changing it's account number to match the bank's format, but also allowing the bank to destroy the account when it's destroyed.

kbw

What is the difference ...

Okay, I understand that passing by reference here would save the program from creating a temporary bank (and a call to the copy constructor), and same goes for the strings; However I'll don't think that explains the bug I'm getting. I will be glad to go back later and change that where I can.


FYI kbw: I'm not a "noob" to programing, Just maybe a little weak in C++ syntax. I've been working with C++ for about a year now, and I've know Java for three to four. Also, I've put a lot of time into programing over that time... Just saying.

[kbw]

It should occur to you that I wouldn't be asking about your standard container rewrites if they were correct.

I'd imagine that each bank makes up its own acocunt numbers. That sort of goes without saying. I guess your model is a little unconventional, it doesn't really matter.

What is the difference ...

This was supposed to make you think seriously about what you've done, but you haven't.

You're getting memory corruptions by not sticking to the rules. If you had stepped through your code as it loaded a file, you'd see where you went wrong.

The copy constuctor and assignment operator signatures are not guidelines or hints. The syntax is specific. You can't just make up your own variants and think that will suffice. It's not a matter of efficiency it's a matter of correctness, which is why you started this thread.

[Mathhead200]

Problem soled! Thanks for the help everyone...
The problem was, because the bank takes ownership of the account pointer, I had no choice but to deep copy the accounts. Otherwise, after the bank destructor was called, the bank would be full of dereferenced pointers. This was causing the new bank to try to dereference them again at the end of the program and... Crash! I guess I over looked the fact that I either needed to deep copy the accounts or remove them from the bank being copied. I went with this solution for now.

Bank::Bank(const Bank &bank) {
	number = bank.number;
	//deep copy accounts.values()
	Array<Integer> accKeys = bank.accounts.keys();
	for( int i = 0; i < accKeys.getSize(); i++ )
		accounts.put( accKeys[i], bank.accounts.get(accKeys[i])->clone() );
	//deep copy names.values()
	Array<String> nameKeys = bank.names.keys();
	for( int i = 0; i < nameKeys.getSize(); i++ )
		names.put( nameKeys[i], new Array<unsigned int>( *bank.names.get(nameKeys[i]) ) );
}

PS: If anyone is still willing, I'm having problems with the load routine. For some reason it's not reattaching the savings accounts. Also I have the encryptionXor function modified to print non-encrypted data for now so I can see the save files.


kbw

You're getting memory corruptions by not sticking to the rules.

No, that's not what happened.

kbw

If you had stepped through your code as it loaded a file, you'd see where you went wrong.

I don't conventionally use an IDE, and had never used a debugger before now. So, thanks for the tip.

kbw

The copy constuctor and assignment operator signatures are not guidelines or hints. The syntax is specific. You can't just make up your own variants and think that will suffice. It's not a matter of efficiency it's a matter of correctness...

Really... It's working okay right now. I believe it is a matter of efficiency. Please give me an example or source before talking to me like I don't understand what I'm doing.

[sohguanh]

Mathhead200, you are not the first nor neither the last to experience the "pain" of C++ programming. Deep-copy, shallow-copy etc are just some of the pitfalls that have tripped up even veteran C++ developers throughout decades.

I believe the Java creators strive to avoid those pitfalls at the expense of some efficiency of cuz but with current hardware acceleration and development, those deficiencies are narrow-ed down even further. Except for mission-critical or time-critical systems, I don't see how C++ should be employed on a large scale business specific system.

Please note I am not anti-C++, in fact I move from C -> C++ (Bea Tuxedo C++ CORBA to be precise) -> Java and after all these years, I come out with this personal opinion.

Nowadays except for exceptional reasons like maintain-ing legacy C/C++ code or identified bottle-neck areas, I pretty much dabble in Java. The reason I am still in this forum is becuz somehow my early roots still hold a place in my heart :)