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.

• Toby Speight: Your platform happens to define size_t in the global namespace as well as in std. This is allowed, but not required; while it works for you, you can't portably depend on that extra definition.
• Danilo: Toby, thank you for response. For all that look upon this comment section, I wrote an follow up question about size_t and Toby answered. I thought comment was unclear and I deleted it without noticing that Toby answered.
• Danilo: So i would need to include standard c definitions everytime i use size_t to maximise portability ?
• Toby Speight: There are six different headers that declare std::size_t - consult your usual reference if you've forgotten which they are. (I generally use CPP Reference, which is also available as a web site.)

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)