Dynamic arrays in functions?

[jc4gavejc]

Hey all, I am having a very strange problem with a program I am writing, and like most other people who post such things on this forum, I cannot figure out what is wrong. Here is my code, please excuse the messiness thereof, and let me point you to where I am having the problem. There is a function (aptly titled "theFunction," that is supposed to, when passed a two dimensional dynamic array (in the form of a double pointer) and a value for the current max size for the array, double the value (called lineLength) and reallocate the original array (called theLines) by initially copying everything over to a temporary array, adjusting the size and reallocating memory for theLines.

I realize that the identifiers in the function are equivalent to their counterparts in main. This is intentional, and if I get this program fixed, this will be changed to reflect a more generic scenario, as convention would have me do. The reason I chose to not rename the identifiers was to demonstrate that if you copy and paste the code from theFunction to before the statement in which it is called, the program functions perfectly. Otherwise, theFunction only runs 3 times, each time converting more of the data into garbage characters that I cannot use. I certainly did not mean to implement such a conversion. Anyway, I am also aware that there are several possible memory leaks and I will deal with them as soon as I can get the rest of the program working. Thanks so much for the help, this is a bit of a last resort for me.

#include <fstream>
#include <iostream>
#include <cstring>

using namespace std;

const int BUF_MAX = 1000;
void nullArray(char*, int);
void nullArray(char**, int);
void sortArray(char **arr, int max);
int theFunction(int lineLength, char ** theLines, int len);
int main()
{
	ifstream fin;   //file input
	int lineCount = 0;
	char buffer[BUF_MAX];
	int lineLength = 10;
	char **theLines = new char*[lineLength];

	//Open input file:
	//cout << "Enter a file path:\n\t==>  ";

	//cin.getline(buffer, BUF_MAX);
	fin.open("C:\\Users\\jc4gavjc\\Downloads\\DynArrayData_1.txt");

	//Check if open failed:
	if(fin.fail())
	{
		cout << "File open failed. Exiting:\n";
		return 1;
	}


	//Loop until end of file:

	int len = 0;
	while(!fin.eof())
	{
		while(len < lineLength)
		{
			fin.getline(buffer, BUF_MAX);
			theLines[len] = new char[strlen(buffer)];
			strcpy_s(theLines[len], strlen(buffer)+1, buffer);
			nullArray(buffer, BUF_MAX);
			len++;
		}
			
		lineLength = theFunction(lineLength, theLines, len);
	}
	sortArray(theLines, len);
	for (int i = 0; i < len; i++)
		cout << theLines[i]<< endl;
	fin.close();
}


void nullArray(char *arr, int max)
{
for(int i = 0; i < max; i++)
	*(arr + i) = '\0';
}

void nullArray(char **arr, int max)
{
for(int i = 0; i < max; i++)
	for(int j = 0; j < max; j++)
		(arr[i])[j] = NULL;
}


void sortArray(char **arr, int max)
{
	char *temp;
	//Iterate through all the lines in the array to find the shortest array:
	for(int i = 0; i < max; i++)
		for(int j = i + 1; j < max; j++)
			if (strlen(arr[i]) > strlen(arr[j]))
			{
				temp = arr[j];
				arr[j] = arr[i];
				arr[i] = temp;
			}
}

int theFunction(int lineLength, char **theLines, int len)
{
	char **temp = new char*[len];
	for(int i = 0; i < len; i++)
	{
		temp[i] = new char[strlen(theLines[i])+1];
		temp[i] = theLines[i];
	}
	lineLength *= 2;
	theLines = new char*[lineLength];
	for(int i = 0; i < len; i++)
	{
		theLines[i] = new char[strlen(temp[i])];
		theLines[i] = temp[i];
	}
	return lineLength;
}

[guestgulkan]

This doesn't look good

    temp[i] = new char[strlen(theLines[i])+1];//allocate an array
    temp[i] = theLines[i];//now totally ignore the array just allocated - and lose the pointer to it as well.
// there is no copying of data taking place -  you are just assigning one pointer to another 

and neither does this (because of the above):

    theLines[i] = new char[strlen(temp[i])];
    theLines[i] = temp[i];

[jc4gavejc]

How does this look?

int theFunction(int lineLength, char **theLines, int len)
{
	char **temp = new char*[len];
	for(int i = 0; i < len; i++)
	{
		temp[i] = new char[strlen(theLines[i])+1];
		strcpy_s(temp[i], strlen(theLines[i]), theLines[i]);
	}
	lineLength *= 2;
	theLines = new char*[lineLength];
	for(int i = 0; i < len; i++)
	{
		theLines[i] = new char[strlen(temp[i])];
		strcpy_s(theLines[i], strlen(temp[i]), temp[i]);
	}
	return lineLength;
}

While this looks good, it actually still doesn't work.
Doing it this way gives me this error:

Debug Assertion Failed!
Program: c:\users\jc4gavjc\Desktop\DYN\Debug\DYN.exe
File: f:\dd\vctools\crt_bld\self_x86\crt\src\tcscpy_s.inl
Line: 30

Expression: (L "Buffer is too small" && 0)

For information on how your program can cause an assertion
failure, see the Visual C++ documentation on asserts.

(Press Retry to debug the application) 

[jc4gavejc]

I did mention that a simple copy paste operation into main causes the program to work perfectly, but I need this in functions.

Any tips at all?

[jc4gavejc]

i'm not asking people to do my assignment, but I am really confused and I really want to put this code into a usable function.

The code I am about to include works perfectly; I just can't write it that way. Writing everything in the main function is bad programming practice, and my professor asked that we do not do so.
Here is some of the random output it gave me.

►▌x
0 h
* Clarity
* Archive
* Efficiency
* Fix errors
* Generality
Phase 5:        Coding
Phase 6:        Testing
* Enhance features
* Validate modules
Phase 8:        Production
Phase 9:        Maintenance
* Develop Interfaces
Phase 4:        Verification
* Divide into Modules
Phase 1:        Specification
Phase 3:        Risk Analysis
Life Cycle of Software
* Bottom-Up vs. Top-Down
Phase 7:        Refining the Solution
* Usually:  "Prove" informally
* Distribute to intended users
* Verify assertions and invariants
For most "larger" software projects,
* Loosely Coupled, Cohesive modularity
--> Phase 9 (Maintenance) comprises 70%.
* Parameters, PreConditions and PostConditions
--> Phases 1-8 comprise 30% of the overall work
* Identify Invariants: Condition that is always true
* Goal: Prove that your solution or algorithm is correct
* Identify, Assess and Manage "risks" associated with the project
* Optional: Produce a Prototype Program that simulates a solution
* Assertions: Statement about a particular condition in an algorithm
Press any key to continue . . .

This is correct except for the garbled bit at the beginning, which I do not know how to correct.

[guestgulkan]

Debug Assertion Failed!
Program: c:\users\jc4gavjc\Desktop\DYN\Debug\DYN.exe
File: f:\dd\vctools\crt_bld\self_x86\crt\src\tcscpy_s.inl
Line: 30

Expression: (L "Buffer is too small" && 0)

For information on how your program can cause an assertion
failure, see the Visual C++ documentation on asserts.

(Press Retry to debug the application) 

This is to do with the strcpy_s function.
Are you using Windows 7??
Strangest thing - I was getting that error as well -but I was testing out my new windows & installation.
tried it in Windows XP and there is no problem

[jc4gavejc]

Nah, running Vista, but I tried it on Ubuntu as well and that just spit out a bunch of garbled nonsense. It is a problem with my code.

I still don't know what it is and the assignment is due tomorrow :(

Please help!

[guestgulkan]

You say that this following version of the function works:

int theFunction(int lineLength, char **theLines, int len)
{
	char **temp = new char*[len];
	for(int i = 0; i < len; i++)
	{
		temp[i] = new char[strlen(theLines[i])+1];
		strcpy_s(temp[i], strlen(theLines[i]), theLines[i]);
	}
	lineLength *= 2;
	theLines = new char*[lineLength];
	for(int i = 0; i < len; i++)
	{
		theLines[i] = new char[strlen(temp[i])];
		strcpy_s(theLines[i], strlen(temp[i]), temp[i]);
	}
	return lineLength;
}

It most probably does, but it does have a glaring memory leak - each time resize the buffer - you copy
the previous data to a temp array (on the heap) - you then copy the saved data to the new array - BUT YOU ARE NOT DESTROYING THE TEMPORARY DATA - you are leaking memory like a sieve.

Also - all this copying is time consuming it would be better if you could copy only once.
Maybe something like this:
**Note the change in theFunction it now takes a reference to a pointer to pointer to char

#include <stdafx.h>
#include <fstream>
#include <iostream>
#include <cstring>

using namespace std;

const int BUF_MAX = 1000;
void nullArray(char*, int);
void nullArray(char**, int);
void sortArray(char **arr, int max);
int theFunction(int lineLength, char ** &theLines, int len);//******************************


int main()
{
    ifstream fin;   //file input
    int lineCount = 0;
    char buffer[BUF_MAX];
    int lineLength = 10;
    char **theLines = new char*[lineLength];

    //Open input file:
    //cout << "Enter a file path:\n\t==>  ";

    //cin.getline(buffer, BUF_MAX);
    fin.open("data.txt");//*** I only changed this name because a had a file with this name already that I could use**

    //Check if open failed:
    if(fin.fail())
    {
        cout << "File open failed. Exiting:\n";
        return 1;
    }


    //Loop until end of file:

    int len = 0;
    while(!fin.eof())
    {
        while(len < lineLength)
        {
            fin.getline(buffer, BUF_MAX);
            theLines[len] = new char[strlen(buffer)+1];//****************************
            strcpy(theLines[len], buffer);//*****************************
            nullArray(buffer, BUF_MAX);
            len++;
        }
        
        lineLength = theFunction(lineLength, theLines, len);

    }
    sortArray(theLines, len);
    for (int i = 0; i < len; i++)
        cout << theLines[i]<< endl;
    fin.close();
}


void nullArray(char *arr, int max)
{
    for(int i = 0; i < max; i++)
        *(arr + i) = '\0';
}

void nullArray(char **arr, int max)
{
    for(int i = 0; i < max; i++)
        for(int j = 0; j < max; j++)
            (arr[i])[j] = NULL;
}


void sortArray(char **arr, int max)
{
    char *temp;
    //Iterate through all the lines in the array to find the shortest array:
    for(int i = 0; i < max; i++)
        for(int j = i + 1; j < max; j++)
            if (strlen(arr[i]) > strlen(arr[j]))
            {
                temp = arr[j];
                arr[j] = arr[i];
                arr[i] = temp;
            }
}



/*!
Function to create a new double size array
*/
int theFunction(int lineLength, char ** &theLines, int len)
{
    char** temp =0;
    
    //Create the double size buffer
    lineLength *= 2;
    temp = new char*[lineLength];
    
    if(!temp)
        return 0;
    
    //allocate the array pointers
    int i =0, count =0;
    for(; i < len; i++)
    {
        //we now try to allocate each array pointer in turn
        //if allocation for a pointer fails - we will delete all 
        //previously allocated pointers
        temp[i] = new char[strlen(theLines[i])+1]();
        if(!temp[i])
        {
            for(; count < i; count++)
            {
                delete [] temp[count];
            }
            delete []temp;
            lineLength=0;
            break;
        }
        else
        {
            //copy old stuff
            strcpy(temp[i],theLines[i]);
        }
        
    }

    //We want to delete the old data 
    if (lineLength !=0)
    {
        for(count =0; count < len; count++)
        {
            delete [] theLines[count];
        }
        delete []theLines;
        theLines = temp;
    }

    return lineLength;
}