C++ problem, please help to explain and fix?

Question

The following code is a 3d dynamic array i try to build to hold some strings, please tell me why can't i print what's assigned to the array? Besides, there's error when trying to clean up the pointer of the array in the end, please help to fix them.

#include
#include
using namespace std;

int main () {
char *str="abcdefghijklmnopqrstuvwxyz";
int x=14,max=2,lcount=0,tcount=0;
char ***arr;

arr=new char**[x];
for(int y=0;y arr[y]=new char*[max];
for(int z=0;z }
while(lcount<2){
strcpy_s( arr[tcount][lcount], 50,str);
tcount++;
cout< if(tcount==13){
lcount++;tcount=0;
}
str++;
}
for(int y=0;y<40;y++){
for(int z=0;z delete [] arr[z][y]; //problem here
delete []arr[y];
}
delete []arr;
system("pause");
return 0;
}

Answer 1

Dear fellow coder,



I'm a programmer with 6 years of industry experience, so I'm probably giving you way more info than you're asking for, but it never hurts to get some useful advice and tips, right?



First off, it is not safe to do a strcpy_s of 50 characters to a buffer size of 40 characters--if str was more than 40 characters long, this would copy over some other memory in your program, and give you a very difficult-to-find memory bug. (If you ever have to solve a memory-stompage bug, there's a memory debugger that you can use--but that's a more advanced topic.)



Second, as a tip, you will have an easier time if you start with a simpler program and work your way up from there.



Third, as a tip, your code will be easier to read if you set x to 13 instead of 14. You will already get the values you need, as 0,1,2,3...,12 already gives you 13 values. As you're probably well aware, programmers start counting at 0 and end at n-1.



Fourth, as a tip, are you compiling on Visual Studio or gcc? If Visual Studio, you should set a breakpoint and use the debugger to debug your program; if gcc, you can use a program called gdb to debug it.



Now, to your bug: Your tcount++ currently happens after your strcopy_s and before your cout. So your string is being copied to a slot, and then the cout is reading from the slot afterwards. What you probably want to do is move your tcount++ to be the line after your cout.



You should get something like:

abcdefghijklmnopqrstuvwxyz

bcdefghijklmnopqrstuvwxyz

cdefghijklmnopqrstuvwxyz

...

xyz

yz

z



If that's not what you're expecting, you can probably change your strcpy_s from copying 50 characters to copying 1 and it would copy a, b, c...x, y, z, like that. If that doesn't work, you would have to manually copy just 1 character and add the 0 character after that, could possibly use sprintf with the format "%1s".



As for your cleaning up problem, your array is being created with these dimensions

arr[14][2]

but being deleted as if it had these dimensions

arr[2][40]



So, your freeing bit of code should look like:



for(int y=0;y
for(int z=0;z
delete [] arr[y][z]; //problem here

delete []arr[y];

}

delete []arr;



Another tip: It will be easier to catch these kinds of bugs if you avoid variable names like x, max, and use more meaningful names like NUM_ROWS, NUM_COLS (all caps because it's a constant).



About the y<40 there, could be a simple mistake or could be a misunderstanding. Understand that arr[y][z] = new char[40]; is allocating a block of 40 characters, but the memory manager remembers the size and location of the chunk of data; so all that's required to free it is delete [] arr[y][z]; --hence, the number 40 doesn't have to show up at all when freeing the data, the memory manager already remembered it was 40 characters long.



So another FYI, if you allocate an array with new, you have to free with delete[], but if you allocate a non-array with new, make sure you delete without the []--like you've done, just an FYI.



Good luck!!

Answer 2

start over fropm scratch and rethink what you are doing. for one thing, your first "new" statement is dubious at best. I am not into twisting my mind around dubious buggy code this late at night.

have you ever thought of using the vector STL type? it is far better than char arrays.

the trouble comes in making it 3d. that takes some forethought, but the end result is far worth it and much more readable code.

sample:

#include

#include

#include

using namespace std;

...

vector vclist;

vector::iterator ivclist;

vclist[0]='a';

vclist.push_back('b');

vclist.push_front('_');

for (ivclist = vclist.begin(); ivclist != vclist.end(); ivclist++) {

cout<<*ivclist;

}



to make larger structures, you will have to make a class.

you can think of an iterator just like a pointer. it can traverse STL structures.

and if you have mingw-w64, you have TR1's mt19937, which is the mersenne twister random number generator!



I think I can safely say it might have something to do with your lack of memory allocation. If you are going to use memory and pointers, it had better be there.



for one thing, your code is really obscure. if you are working with an alphabet, I suggest that you use <= 13 instead of < 14 it is much more readable that way. it also shows you your logic errors. for instance, there are 26 letters. you are counting from 0 to 13, a total of 14 indexes (characters). and you I think are doing it twice to cover the range of the alphabet. this is a classic "off-by-1" error.



also, you allocate 40 characters "new char[40];" but when you do the strcpy_s, you use the number 50 - that concerns me that maybe you are causing the array to go out of bounds. I don't have documentation on that function. it's not in mingw I don't think.



you should turn on compiler stack checking and see what happens to your program. in gcc it is -fstack-check. then re-execute your program and see what it does. there are other compiler options. also, with gcc I usually turn on all warnings. -W -Wall.

Answer 3

Take another look at these three statements. Can you see the problem yet?



strcpy_s( arr[tcount][lcount], 50,str);

tcount++;

cout<


Have you got it now?



Have fun.