locked
Deleting Two Dimensional Array RRS feed

  • Question

  • I'm getting a memory error from deleting a previously dimensioned array

    I dimension it by increasing the value and using NEW:

         totkeys++;

         std::string **tplkey;
         tplkey = new std::string*[totkeys];

         for(i=0;i<=totkeys;i++)
         {
           tplkey[i] = new std::string[3];
         }

         totkeys--;

    then after I'm done with it, when the function ends I try to delete the array:

         totkeys++;
        
         for(i=0;i<=totkeys;i++)
         {
           delete [] tplkey[i];
         }
        
         delete [] tplkey;

    Is there anything that is evident that I'm doing wrong here?


    • Edited by TallGuy63 Monday, October 5, 2015 6:09 PM
    Monday, October 5, 2015 6:08 PM

Answers

  • Change your conditionals from <= to < only:

    //for(i=0;i<=totkeys;i++)
    for(i=0;i<totkeys;i++)

    - Wayne
    • Marked as answer by TallGuy63 Monday, October 5, 2015 10:09 PM
    Monday, October 5, 2015 6:29 PM
  • Go back and read Wayne's response again.  C++ uses zero-based array indices.  Both for loops cause undefined behavior.

    You allocate space for FIVE pointers to string.  It is legal to reference these pointers with the expression tplkey[i] ONLY when i has a value of 0, 1, 2, 3. or 4.

    You then attempt to assign values to SIX such pointers.  Your for loop attempts to assign values to tplkey[i] when i has values 0, 1, 2, 3, 4, AND 5.  That last one attempts to access a non-existent pointer (probably in memory you don't own).

    Once you overrun allocated memory, you have no idea what the program will do.  Consider yourself lucky that the system was able to give you some kind of diagnostic informing you of the problem.

    • Marked as answer by TallGuy63 Monday, October 5, 2015 10:08 PM
    Monday, October 5, 2015 9:00 PM

All replies

  •      totkeys++;


         std::string **tplkey;
         tplkey = new std::string*[totkeys];

         for(i=0;i<=totkeys;i++)
         {
           tplkey[i] = new std::string[3];
         }

         totkeys--;

    ...

         totkeys++;
        
         for(i=0;i<=totkeys;i++)
         {
           delete [] tplkey[i];
         }
        
         delete [] tplkey;



    How and where is totkeys defined?

    What is it's initial value before you do totkeys++ for the allocations?

    What is it's initial value before you do totkeys++ for the deallocations?

    Use the debugger to step through your code and watch the values of totkeys at each step.

    - Wayne

    • Marked as answer by TallGuy63 Monday, October 5, 2015 10:09 PM
    • Unmarked as answer by TallGuy63 Monday, October 5, 2015 10:09 PM
    Monday, October 5, 2015 6:20 PM
  • Change your conditionals from <= to < only:

    //for(i=0;i<=totkeys;i++)
    for(i=0;i<totkeys;i++)

    - Wayne
    • Marked as answer by TallGuy63 Monday, October 5, 2015 10:09 PM
    Monday, October 5, 2015 6:29 PM
  • The values are variable but in this case totkeys is 22

    The values are correct; that was the first thing I checked...

    Monday, October 5, 2015 6:56 PM
  •  

    I tried changing both the dimensioning and and the deleting to "i < totkeys" and it gave the same memory error.  But this does bring up another question; I noticed you're supposed to add one to the value before dimensioning it for some reason but it never uses the extra dimension, so is there another reason for that?

    I should also note that I get no memory error when I simply don't delete the array when I'm done with it... I'm using Visual Studio 2013 and compiling in 64-bit



    • Edited by TallGuy63 Monday, October 5, 2015 7:07 PM
    Monday, October 5, 2015 7:02 PM
  • When you use <= you are allocating to, and trying to deallocate, one more element than
    exists in the array.

    If the array size is 22 then you can access/use indexes 0-21, not 0-22.

    If you are still getting errors you need to post the *exact* code (copy & paste) as well
    as the *exact* error (copy & paste if possible or post an image).

    The code you posted originally will *not* cause a memory error if you change the conditionals
    as I suggested. But you may have other access errors in the code you haven't posted, which
    uses the array between allocation and deallocation.

    - Wayne

    Monday, October 5, 2015 7:09 PM
  •  

    I noticed you're supposed to add one to the value before dimensioning it for some reason but it never uses the extra dimension, so is there another reason for that?

    There is no such general requirement. You may need to do that in cases where you are
    allocating space to hold a C string, and you know the number of characters in the string
    (strlen) but you have to add one to the allocation to make room for the terminating nul
    character.

    - Wayne

    Monday, October 5, 2015 7:20 PM
  •   I should also note that I get no memory error when I simply don't delete the array when I'm done with it...


    The fact that the debugger or the RTL or the OS doesn't throw up after your program ends
    doesn't mean that you aren't overwriting memory when it's running. Some of the checks for
    heap corruption take place during the housekeeping associated with deleting/freeing memory.

    - Wayne

    Monday, October 5, 2015 7:24 PM
  • I can't post all the code; this is just a tiny portion of it; the program compiles to over 6 MB; the code is source code is stored in about 100 files... I'll just keep trying to figure it out; so far the issue has completely eluded me though.

    Monday, October 5, 2015 8:02 PM
  • If you want to see the heap corruption and the memory leak from from not deleting the array run

    #include "stdafx.h"
    #include <crtdbg.h>
    #include <string>
    
    int main()
    {
    	int totkeys = 5;
    
    	std::string **tplkey;
    
    	_CrtSetDbgFlag(_CRTDBG_LEAK_CHECK_DF | _CRTDBG_ALLOC_MEM_DF | _CRTDBG_CHECK_ALWAYS_DF);
    
    	tplkey = new std::string*[totkeys];
    
    	for (int i = 0; i <= totkeys; i++)
    	{
    		tplkey[i] = new std::string[3];
    	}
    
    	for (int i = 0; i <= totkeys; i++)
    	{
    		delete[] tplkey[i];
    	}
    
    	//delete[] tplkey;
    
        return 0;
    }
    

    Monday, October 5, 2015 8:29 PM
  • Go back and read Wayne's response again.  C++ uses zero-based array indices.  Both for loops cause undefined behavior.

    You allocate space for FIVE pointers to string.  It is legal to reference these pointers with the expression tplkey[i] ONLY when i has a value of 0, 1, 2, 3. or 4.

    You then attempt to assign values to SIX such pointers.  Your for loop attempts to assign values to tplkey[i] when i has values 0, 1, 2, 3, 4, AND 5.  That last one attempts to access a non-existent pointer (probably in memory you don't own).

    Once you overrun allocated memory, you have no idea what the program will do.  Consider yourself lucky that the system was able to give you some kind of diagnostic informing you of the problem.

    • Marked as answer by TallGuy63 Monday, October 5, 2015 10:08 PM
    Monday, October 5, 2015 9:00 PM
  • B-S,

    I know the code is bad.  I used the OP's code but inserted the debugging specification to reveal the problems.

    For example, he didn't seem to think there was a memory leak of he didn't delete the allocation contained in the tplkey pointer.

    Monday, October 5, 2015 9:03 PM
  • Thanks... I think you nailed it...

    I didn't even realize I was dimensioning something that wasn't there...

    Monday, October 5, 2015 10:04 PM
  • I didn't even realize I was dimensioning something that wasn't there...

    Even after I explained it to you in detail? I said:

    "When you use <= you are allocating to, and trying to deallocate, one more element than
    exists in the array.

    If the array size is 22 then you can access/use indexes 0-21, not 0-22."

    Seems clear enough to me.

    - Wayne

    Monday, October 5, 2015 10:15 PM
  • What can you say when you don't *see* something??

    I didn't see it.  Sorry.  It made sense to me the way it was.

    In my 25+ years of programming this still happens occasionally-- especially since switching to this new language. I'm not going to be ashamed of the fact that I didn't see it; I just didn't.  Thanks for helping; I appreciate your input.

    Monday, October 5, 2015 10:21 PM
  • One last comment if I may: Doing dynamic memory management using new/delete, new[]/delete[].
    malloc/free. etc. is considered "old school" now and best advice is to avoid them whenever
    possible. They are notorious for letting leaks, overruns, etc. sneak in, even on the most
    careful programmer. Try to use containers such as std::vector, std::list, etc. whenever
    possible, as they manage all the "dirty details" of allocations, deletions, reallocations,
    etc. for you.

    - Wayne
    Monday, October 5, 2015 10:28 PM