5

First time with an account here but I've enjoyed the site a fair deal.

I'm attempting to create a function that receives a const char array and returns a portion indicated of said array. Along with the array the function receives two values indicating the index of the first character in the portion to be extracted and the index of the last character.

The tricky part about what I'm attempting is that I'm creating a temporary array variable to hold this portion inside a function and given that the size of the portion isn't guaranteed to be a constant I'm using dynamic memory to allocated the necessary space and here's where I'm having my issue.

Whenever the function returns any information the function ends and the program doesn't have a chance of deallocating the spared memory. If I delete the variable I'm unable to return the information.

I tried creating a separate pointer variable to point to the information after it's formed but once the memory is deallocated the information doesn't seem to be able to be recovered.

Program with issues dealllocating:

char* seperateString(const char* a, int b, int c) {

                // This is simply to assure that I don't allocated a negative size
        if (b < c) {
                cout << "***Error with \"seperateString\" function***" << endl << endl;
                return '\0';
        }

        char* seperated = new char[c - b + 2];

        int i = 0;
        int j = b;

        for (; i <= c - b; i++)
                seperated[i] = a[j++];

        seperated[i] = '\0';

        char* send = seperated;
        delete[] seperated;

        return send;
}

int main() {
        cout << "Program Iniciated." << endl << endl;

        const int a = 6, b = 11;


        const char* toBeSeperated = "Hello there, I have missed you.";
        char *ari = seperateString(toBeSeperated, 6, 11);

        cout << "Sentence is: " << toBeSeperated << endl << endl
            << "Extracted portion is: " << ari << endl << endl;

        cin.get();
        return 0;
}

Program working as intended in the main function.

int main() {
        cout << "Program Iniciated." << endl << endl;

            // variable a and b hold the beginning and last index values of the portion 
            // that is wanted to extract. 
        const int a = 6, b = 11;


            // This is an example sentence  |------| this is the portion being extracted.
        const char* toBeSeperated = "Hello there, I have missed you.";

            // variable holding extracted portion.
            // created with the size necessary to hold the portion plus a \0.
        char* seperated = new char[b  -a  +2];
                                                                //Given that a and b are index values 1 has to be added for size
                                                                //Additionally an extra space is require for \0

            //i is held outside the for loop to allow to access it after it completes the cycle
          //so it's able to assing \0 after the last character.
        int i = 0;

          //j holds the value of the first index
        int j = a;

        for (; i <= b - a; i++)
                seperated[i] = toBeSeperated[j++];

        seperated[i] = '\0';

        cout << "Sentence is: " << toBeSeperated << endl << endl
                << "Extracted portion is: " << seperated << endl << endl;

        delete[] seperated;
        cin.get();
        return 0;
}

In closing, this is about preventing memory leakage.

t.niese
  • 39,256
  • 9
  • 74
  • 101
Pichuu64
  • 83
  • 6
  • 1
    Sounds like a great job for a smart pointer (such as [`std::unique_ptr`](https://en.cppreference.com/w/cpp/memory/unique_ptr)) But unfortunately I'm too unfamiliar with them to create an example in an answer. –  Aug 06 '19 at 04:07
  • 2
    `seperateString` returns a pointer to deleted memory. Consider having the function return `std::string` instead. – jkb Aug 06 '19 at 04:08
  • 1
    `return seperated;` and then delete in the caller when you are done with it (old C/C++ way of doing things). Use `std::string` and use `.substr()` to extract between indexes and return and let the compiler worry about the lifetime of the object. [std::basic_string](https://en.cppreference.com/w/cpp/string/basic_string) – David C. Rankin Aug 06 '19 at 04:32
  • Simple rules: Do never use raw pointers, especially not for owned memory. Do not use new or malloc or something similar. Do not use C-Style arrays. For strings, do not use char*. Use ````std::string````. Then, automatically, you will not have such problems. And, your task can be simply achieved with the ````substr```` function – A M Aug 06 '19 at 04:40
  • @ArminMontigny raw pointer (as long as observer pointer are not finalized) should still be used. You will use them (or references to the object) whenever you use an object without passing its ownership (so for all your utility functions). But you should not do manual memory management using `new`,`delete`,`malloc`,`dealloc`, ... . – t.niese Aug 06 '19 at 05:02
  • 1
    Use std::string. std::string is the right tool for the job. Whenever you are dealing with strings, std::string is the first thing you should consider. Not using std::string is counterproductive. – n. m. could be an AI Aug 06 '19 at 05:19

3 Answers3

7

As you pointed out, we must have memory that actually stores the result of the function, so we can't free the dynamically allocated array before we return. That leaves us with two options:

  1. Allocate the memory before we call, and provide a pointer to and size of that memory to the called function (less than elegant)
  2. Make sure that we pass back a pointer to the allocated memory in a safe way

In both cases, however, we have to make sure to manually deallocate the memory. Smart pointers are perfectly suited for this use-case, specifically std::unique_ptr.

Here's a functioning program:

#include <iostream>
#include <memory>

std::unique_ptr<char[]> seperateString(const char* a, int b, int c) {
    using namespace std;
    // This is simply to assure that I don't allocated a negative size
    if (c < b) {
        cout << "***Error with \"seperateString\" function***" << endl << endl;
        return '\0';
    }

    auto seperated = std::unique_ptr<char[]>(new char[c - b + 2]);

    int i = 0;
    int j = b;

    for (; i <= c - b; i++)
        seperated[i] = a[j++];

    seperated[i] = '\0';

    return seperated;
}

int main() {
    using namespace std;
    cout << "Program Iniciated." << endl << endl;

    const int a = 6, b = 11;


    const char* toBeSeperated = "Hello there, I have missed you.";
    auto ari = seperateString(toBeSeperated, 0, 5);

    cout << "Sentence is: " << toBeSeperated << endl << endl
        << "Extracted portion is: " << ari.get() << endl << endl;

    cin.get();
    return 0;
}

Unique pointer takes ownership of the dynamically allocated resource, and releases (i.e. deallocates) the resource when it itself is destroyed on leaving scope.

In function seperateString, we construct the unique_ptr with a pointer to a dynamically allocated array of characters via operator new[]. From this point on, we don't need to manage the dynamically allocated memory as it is bound to the lifetime of the smart pointer seperated. Whenever seperated is destroyed, its destructor will call operator delete[] on the array pointer we assigned to it upon construction.

But wait, seperated is destroyed as the function returns, so aren't we back to square one as the memory is freed before we get to use it in the calling code? No, because we are returning the unique_ptr<char[]> by value, the compiler moves ownership of the resource from the function-local seperated unique pointer to the call-site-local ari unique pointer, constructing it via a move-constructor. Now the fate of the memory allocated in the function call is tied to the lifetime of ari, and when it goes out of scope the memory is freed with no leaks.

Skrino
  • 520
  • 2
  • 12
  • Maybe better ````std::make_unique````? Should not use new . . . – A M Aug 06 '19 at 04:34
  • 1
    @ArminMontigny it's tagged as C++11, and `std::make_unique` was introduced in C++14 IIRC – Skrino Aug 06 '19 at 04:36
  • Thank you very much. Actually gives some clarity to the issue as a whole. – Pichuu64 Aug 06 '19 at 18:11
  • Any chance you could point out if my approach is misguided? I know there's fuctions utilizing string but I'm attempting to get familiar with char arrays and would rather not utilize them. Only system features if possible – Pichuu64 Aug 06 '19 at 18:14
  • Could I use the function to return a pointer to the memory allocated, use it and delete it after like this:. Newptr = separetedString() and then delete [] Newptr? That should avoid memory leakage right? – Pichuu64 Aug 06 '19 at 18:31
  • Yes that will work, but only as long as you **remember** to call `delete[]`, and writing code that relies on somebody remembering to do something to avoid a memory leak is _highly_ discouraged and a very bad practice to get into. Smart pointers exist to do this for you automatically -- to remove the need for _remembering_ to release memory -- and should always (in 99.99% of cases) be used with dynamically allocated memory instead of raw pointers. – Skrino Aug 06 '19 at 18:38
4

You just reimplemented std::string::substr() for no good reason.

You can delete your function altogether, replace your const char* strings with std::string, and replace calls to your old function with calls to std::string::substr():

std::string toBeSeperated = "Hello there, I have missed you.";
auto ari = toBeSeperated.substr(0, 5);
Nikos C.
  • 50,738
  • 9
  • 71
  • 96
  • Thank you, I need to read more documentation definitely but it still leaves me with the doubt if allocating data to be returned isn't possible in a function the way I'm wishing to use it. This might just be an example but it could come up in a different project I might have. – Pichuu64 Aug 06 '19 at 18:09
  • @Pichuu64 Doing manual memory allocation in C++ is almost never needed nowadays. Use smart pointers for that. Basically, it is possible to write C++ without any calls to `new` and `delete`. – Nikos C. Aug 06 '19 at 18:32
  • @nNikos C. Thanks, yeah I'm noticing the advantages of utilizing premade libraries – Pichuu64 Aug 07 '19 at 19:23
2

The below code usesSTL containers in C++ and these will be automatically managed by C++, Read this to know more. You can also use SMART Pointers. You can also do in this way to extract the sub-string.

#include<iostream>
#include<vector>
#include<memory>

using namespace std;

std::vector<char> seperateString(const char* a,const int b,const int c) {

        std::vector<char> extracted_array;
        for(int i=b;i<=c;i++ ) extracted_array.push_back(a[i]);
        return extracted_array;
}

int main(){
        cout << "Program Iniciated." << endl << endl; // Initiated not Iniciated, Lol.

        int a = 11, b = 0; // here 'const' restricts your freedom to modify them.


        const char* toBeSeperated = "Hello there, I have missed you.";
        auto after_extraction = seperateString(toBeSeperated, a, b);

        for(const auto i : after_extraction) cout<<i;  // to print the extraxcted portion;
        return 0;
}