Possible memory leak in using dynamic memory?

Question

In this program, I coded to get a list of names, specifically using pointers, and perform bubble sort and binary search on the list. I recognize that there are better ways of dealing with this type of memory, but I want to learn this way properly before moving on. I was having errors with not properly deallocating and/or allocating memory, but now, I don't have any error messages popping up running this code, though I still believe that some memory has not been properly deallocated. Here's my code:

Main.cpp

#include "Functions.h"
void main() {
     // initialize variables
     int numNames = 0;
     char* search;
     int index = 0;
     bool isSearching = true;
     // get list of names
     cout << "Enter your list of names:" << endl;
     char** nameList = readList(numNames);
     // sort list of names
     bubbleSort(nameList, numNames);
     // show sorted list of names
     cout << "Sorted list of names:" << endl;
     for (int i = 0; i < numNames; i++) {
         cout << nameList[i] << endl;
     }
     // do binary search loop
     do {
         // get search word
         cout << "Enter a word you would like to search for: " << endl;
         search = readLine();
         // get index of search
         index = binarySearch(nameList, numNames, search);
         // if missing
         if (index == -1) {
             cout << "That word is missing" << endl;
         }
         // show index
         else {
            cout << "That word is at index " << index << endl;
         }
         // set loop test
         isSearching = search[0] != '\0';
         // free mem
        delete[] search;
     } while (isSearching);
     // free mem
     for (int i = 0; i < numNames; i++) {
        delete[] nameList[i];
     }
     delete[] nameList;
     system("Pause");
}

Functions.h

#ifndef FUNCTIONS_H
#define FUNCTIONS_H

#include "ReadFunctions.h"
#include "SortingAndSearchingFunctions.h"

#endif

ReadFunctions.h

#ifndef READ_FUNCTIONS_H
#define READ_FUNCTIONS_H

#include <iostream>
#include <conio.h>
#include <memory.h>
#include "Functions.h"

using namespace std;

char* readLine();
char** readList(int &);

#endif

ReadFunctions.cpp (the readList function was having problems before, and although errors are not popping up, I still think that there may be a memory leak.)

#include "ReadFunctions.h"

static void backspace() {
    _putch('\b');
    _putch(' ');
}

char** readList(int & numWords) { // read list of words
    // initialize variables
    const int firstNumWords = 10;
    char* word;
    numWords = 0;
    int maxWords = firstNumWords;
    char** wordList = new char*[maxWords];
    char** temp;
    // read list loop
    while ((word = readLine())[0] != '\0') {
        // add word
        wordList[numWords++] = word;
        // expand list
        if (numWords >= maxWords) {
            maxWords += firstNumWords;
            temp = new char*[maxWords];
            memcpy(temp, wordList, numWords * sizeof(char*));
            wordList = temp; 
            //may need delete statement?
        }
    }
    return wordList;
}

char* readLine() { // read word
    // initialize variables
    const int firstNumChar = 10;
    char c;
    int charsEntered = 0;
    int maxChars = firstNumChar;
    char* word = new char[maxChars + 1];
    char* temp;
    cout << "> ";
    // read word loop
    while ((c = _getch()) != '\r') {
        // backspace
        if (c == '\b') {
            if (charsEntered == 0) {
                c = '\a';
            }
            else {
                backspace();
                charsEntered--;
                word[charsEntered] = '\0';
            }
        }
        else {
            // add character
            word[charsEntered++] = c;
            // expand word
            if (charsEntered >= maxChars) {
                maxChars += firstNumChar;
                temp = new char[maxChars + 1];
                memcpy(temp, word, charsEntered);
                delete[] word;
                word = temp;
            }
        }
        _putch(c);
    }
    _putch('\n');
    word[charsEntered] = '\0';
    return word;
}

SortingAndSearchingFunctions.h

#ifndef SORTING_AND_SEARCHING_FUNCTIONS_H
#define SORTING_AND_SEARCHING_FUNCTIONS_H

#include "Functions.h"
#include <string.h>

void bubbleSort(char** &, int);
int binarySearch(char** &, int, char*);

#endif

SortingAndSearchingFunctions.cpp

#include "SortingAndSearchingFunctions.h"

static bool isAfter(char* str1, char* str2) { // check if first string is after the second string
    // initialize variables
    bool isGreater = false;
    bool isEqual = false;
    char c1;
    char c2;
    int compareLen = (strlen(str1) >= strlen(str2)) ? strlen(str2) : strlen(str1);
    // check c-strings
    for (int i = 0; i < compareLen; i++) {
        // get compare characters
        c1 = str1[i];
        c2 = str2[i];
        // check if both alphabetical
        if (((c1 >= 'a' && c1 <= 'z') || (c1 >= 'A' && c1 <= 'Z')) && ((c2 >= 'a' && c2 <= 'z') || (c2 >= 'A' && c2 <= 'Z'))) {
            // normalize for case insensitive
            if (c1 >= 'A' && c1 <= 'Z') {
                c1 = (char)(c1 + 32);
            }
            else if (c2 >= 'A' && c2 <= 'Z') {
                c2 = (char)(c2 + 32);
            }
        }
        // handle comes after
        if (c1 > c2) {
            isGreater = true;
            isEqual = false;
            i = compareLen;
        }
        // handle comes before
        else if (c1 < c2) {
            isGreater = false;
            isEqual = false;
            i = compareLen;
        }
        // handle equal
        else {
            isEqual = true;
        }
        // case with symbol, which case to compare with?
    }
    // handle equal comparison region
    if (isEqual) {
        if (strlen(str1) > strlen(str2)) {
            isGreater = true;
        }
    }
    return isGreater;
}

void bubbleSort(char** & wordList, int numWords) { // bubble sort for word lists
    // initialize variables
    char* temp;
    bool swap = false;
    // sort loop
    do {
        // set swap and numwords
        swap = false;
        numWords--;
        // check list
        for (int i = 0; i < numWords; i++) {
            // swap
            if (isAfter(wordList[i], wordList[i + 1])) {
                swap = true;
                temp = new char[((strlen(wordList[i]) >= strlen(wordList[i + 1])) ? strlen(wordList[i + 1]) : strlen(wordList[i])) + 1];
                memcpy(temp, wordList[i], strlen(wordList[i]) + 1);
                //temp = wordList[i];
                memcpy(wordList[i], wordList[i + 1], strlen(wordList[i + 1]) + 1);
                //wordList[i] = wordList[i + 1];
                memcpy(wordList[i + 1], temp, strlen(temp) + 1);
                //wordList[i + 1] = temp;
                //delete[] temp; // if included shows access violation, but should still include and find another way? 
            }
        }
    } while (swap);
}

int binarySearch(char** & wordList, int numWords, char* search) { // binary search for word lists
    // initialize variables
    int first = 0;
    int mid;
    int last = numWords - 1;
    // search loop
    do {
        // set middle index
        mid = (first + last) / 2;
        // check if equal
        if (strcmp(search, wordList[mid]) == 0) {
            return mid;
        }
        // check if after
        else if (isAfter(search, wordList[mid])) {
            first = mid + 1;
        }
        // check if before
        else {
            last = mid - 1;
        }
    } while (first <= last);
    // not found
    return -1;
}

If you don't find any problems, is there any way that I can properly set up dynamic memory and check for memory leaks so that I don't have any worries about this problem.

EDIT: Here's a modified version of the code in the while loop in the readList function in the ReadFunctions.cpp file:

// add word
    wordList[numWords++] = word;
    // expand list
    if (numWords >= maxWords) {
        maxWords += firstNumWords;
        temp = new char*[maxWords];
        memcpy(temp, wordList, numWords * sizeof(char*));
        delete[] wordList;
        wordList = temp; 
    }

Nothing is deleted when I use the new statement with the temp double pointer variable in the readList function I've modified above. When I modified it though, an error message popped up saying there was an access violation into 0xCDCDCD. — Peter Cardenas, Jul 20 '17 at 04:10
IIRC an access violation when calling `delete` probably means you've already used `delete` on that variable. There's [no way to check if a variable has been deleted](https://stackoverflow.com/questions/16050793/how-do-i-know-if-a-pointer-has-been-assigned-data-via-new), so you either have to carefully design your code so you never accidentally delete an already-deleted variable, or assign it `null` after deleting it and then always check if it's `null` or not before calling `delete` on it. — Clonkex, Jul 20 '17 at 04:15

Michaël Roy · Accepted Answer · 2017-07-20T04:50:15.327

0

What's keeping you from using std::vector and std::string? This would eliminate all memory leaks, make the code more efficient, and reduce the size of you code quite a bit. Using std::sort would also help reduce the unnecessary complexity of the code.

And you do need a delete[] statement around the //may need delete statement? comment.

Your modified code doesn't work, because :

if (numWords >= maxWords) {
    maxWords += firstNumWords;
    temp = new char*[maxWords];
    for (int i = 0; i < numWords; i++) {       // you're copying into uninititialized pointers !!!!!
        memcpy(temp[i], wordList[i], (strlen(wordList[i]) + 1) * sizeof(char));
    }
    for (int i = 0; i < numWords; i++) {
        delete[] wordList[i];
    }
    wordList = temp; 
}

The proper way to resize an array of pointers:

if (numWords >= maxWords) {
    temp = new char*[maxWords + firstNumWords];        // allocate new array
    memcpy(temp, wordList, maxWords * sizeof(char*));  // copy the pointer values.
    char* p = wordList;
    wordList = temp;                                   // put new array in place.
    delete[] p;                                        // delete old array.
    maxWords += firstNumWords;                         // fix size var.
}

edited Jul 20 '17 at 04:50

answered Jul 20 '17 at 02:29

Michaël Roy

6,338
1
15
19

This is for an assignment for a class, and the requirements were to use pointers and no libraries that we haven't covered yet. And are you saying that the temp variable would need the delete statement? – Peter Cardenas Jul 20 '17 at 03:03
This would mean having to loop through the double pointer to delete all the contents correct? – Peter Cardenas Jul 20 '17 at 03:13
No, since you copied the contained pointers, but the spent wordList needs to be deleted. – Michaël Roy Jul 20 '17 at 03:16
I've modified code above and got an access violation error into 0xCDCDCD. Am I deleting improperly, copying memory improperly or something else? – Peter Cardenas Jul 20 '17 at 04:12
My teacher doesn't allow us to use swap, could you show a way without using that function? – Peter Cardenas Jul 20 '17 at 04:44
I've modified the code above again, no errors seem to pop up, but do you see anything wrong with it? Thanks for helping me with my problem. – Peter Cardenas Jul 20 '17 at 04:59
You forgot to delete nameList[] before exiting the program. – Michaël Roy Jul 20 '17 at 05:08
Ok, fixed, there seems to not be any more holes. Thanks for your help. – Peter Cardenas Jul 20 '17 at 05:12

Possible memory leak in using dynamic memory?

1 Answers1