Replace part of the string with n size with string of m size

Question

I wanted to understand memory and pointers better before I endeavour into file path manipulations for larger project. This is simple replace part of the string test function that I need to rewrite into struct to call upon. I used scopes to test for memory leaking and changes, I work on Windows 7, CygWin and notepad++.

I am using std::size_t as an size representation due to:

A type whose size cannot be represented by std::size_t is ill-formed mentioned by cppreference with "std::size_t is commonly used for array indexing and loop counting".

I decided to go for the c-style null terminated character array since arguments passed through main(int argc, char **argv) are null terminated characters and system calls accept those best. It seemed beneficial to code parts that i need myself instead of relying on another library, and it is tiresome to keep track what is converted with c_str() and what isn't.

As mentioned above, this function is part of project that manipulates, changes and makes full or relative paths to the files. The character array helps me use stat that comes with CygWin to be able to automatically test if path is not accessible, directory or file. I could use std::string but that is an extra header and i plan serious expansions of file paths to include some custom behaviour - which isn't tied to this function.

#include <iostream>

size_t get_str_size(const char* temp){
            size_t s=0;
            while( temp[s] != '\0' )s++;
            return s;
        }

int main(int argc, char **argv){ 

    const char * temp = "sublime/subliminal/sfit.exe"; // example string
    std::cout << &temp << "\t" << temp << std::endl; // prints out address and value
    { // wild scope to help me navigate and debug
        const char* shitcake = "sfitcake"; // string to be put in place of sfit
        size_t ldelim = 0;  // / last delimiter 
        size_t edelim = 0;  // . end delimiter
        size_t original_size = get_str_size(temp);

        // finding the necessary indexes of charachters  
        for( int i =0 ; i <= original_size; i++ ){
                if( temp[i]=='/' ){ldelim=i;}
                if( temp[i]=='.' ){edelim=i;}
            }

        // calculating size that influences the char array
        size_t old_size = edelim - ldelim -1;  // current char array size(-1 to remove last delimiter)
        size_t new_size = get_str_size(shitcake); // new char array size

        // char array duplication , since original string is const
        char* tempcopy = new char[original_size]; // holds the copy of the temp
        for( int i =0 ; i < original_size; i++ ){ tempcopy[i] = temp[i]; }

        // value to be constructed
        char *new_version = new char[ original_size + (new_size-old_size) ];

        // filling the part before replacement happen       
        for( int i =0 ; i <=ldelim; i++ )               { new_version[i] = tempcopy[i]; }
        // filling replaced char array
        for( int i =0 ; i <= new_size;i++ )             { new_version[i+1+ldelim] = shitcake[i]; }
        // the rest of original string
        for( int i =edelim ; i < original_size;i++ )    { new_version[i+old_size] = tempcopy[i];}

        //conversion back to const char* with expanded size and new value
        temp = (const char*)new_version;

        //deleting pointers
        delete tempcopy;
        delete new_version;
    }
    std::cout << &temp << "\t" << temp << std::endl;

    return 0;
}

The output i am getting is:

0xffffcbb8  sublime/subliminal/sfit.exe
0xffffcbb8  sublime/subliminal/sfitcake.exe

Which means it works, but i am afraid of pointer not changing memory address even though the size and value changed.

Main concerns and desires for code:

• Removing chance of dangling pointer and memory issue
• Fast and readable code

Answer 1

There's no definition in scope for size_t. Since the description implies that you want std::size_t, you'll need

#include <cstddef>
using std::size_t

But I recommend just writing std::size_t in full where you use it - that's much clearer to readers.

---

size_t get_str_size(const char* temp){
            size_t s=0;
            while( temp[s] != '\0' )s++;
            return s;
        }

You've just re-implemented std::strlen() for no good reason.

---

    char* tempcopy = new char[original_size]; // holds the copy of the temp

    char *new_version = new char[ original_size + (new_size-old_size) ];

    delete tempcopy;
    delete new_version;

Memory allocated with new[] must be released using delete[] (not delete). Valgrind will catch this error for you.

---

We have a memory leak if the first allocation succeeds and the second fails, because there is no delete[] tempcopy if an std::bad_alloc is thrown.

---

We reference deleted memory here:

    temp = (const char*)new_version;

    delete new_version;

    std::cout << &temp << "\t" << temp << std::endl;

That's Undefined Behaviour (and also caught by Valgrind - I really recommend you run your code under Memcheck, or any similar tool of your choice).

---

int main(int argc, char **argv){

Since we never use the arguments, there's no need to give them names; moreover, there's a signature of main() we can use with no arguments:

int main()

---

     for( int i =0 ; i <= original_size; i++ ){

Avoid comparing signed values (such as i) against unsigned ones (such as original_size). We probably want the range of i to match, too, so just use the same type for both.

---

The code isn't very robust against a range of inputs. In particular, if edelim is less than ldelim, we'll get (unsigned) integer overflow when we compute old_size, giving incorrect results.

---

Overall, the code looks very much like C code - and poor C code at that (e.g. copying 1 char at a time, rather than with memcpy()). A C++ implementation (using std::string) is much shorter and more natural.

Answer 2

You tagged this C++, but your code does not at all look like C++. It looks like C with malloc replaced by new and printf replaced by std::cout. You can use C++ standard library facilities to simplify your code. In this case, you can simplify your code with std::string:

#include <string>
#include <string_view>

std::string replace(std::string_view string, std::string_view pattern, std::string_view replace)
{
    std::string result;

    while (true) {
        auto index = string.find(pattern);
        result.append(string, 0, index);
        if (index == string.npos)
            break;
        string.remove_prefix(index + pattern.size());
        result += replace;
    }
    return result;
}

(live demo)