memcpy failure

[sajis997]

Hello forum,

I am having a segmentation fault with the following memcpy operation:

   const char *binaryExt = std::string(".bin").c_str();
   const char *fileName = std::string("main.cl").c_str();
   char *binaryFileName = NULL;
   

   std::memcpy(binaryFileName,fileName, strlen(fileName)+1);

Any idea?

Regards
Sajjad

[Disch]

char pointers are not strings... they're pointers.

your 'binaryFileName' var does not point to anything... it is a null pointer.

Your memcpy line is trying to copy data to a null pointer. Writing to a null pointer often causes a crash.

You need to make your pointer actually point to something.


Furthermore....

const char *binaryExt = std::string(".bin").c_str();

You are creating a temporary std::string object here... then calling c_str() to get a pointer to that object's data.

Since the object it temporary... it goes out of scope (dies) immediately after this line. Which means that the string data you're pointing to dies with it. Therefore, your binaryExt pointer points to random memory, and not the string data you think it does.


So basically all of your pointers here are bad and messed up.


I recommend not using functions like memcpy or char pointers/buffers until you understand what pointers actually are. using std::string is much easier and safer.

[Disch]

Let me try and explain a little further.

String data needs [relatively] a lot of space. Each character in the string needs a char. Additionally, for C-style strings (char arrays), you need to end the string data with a literal 0.. known as the "null terminator" to mark the end of the string.

So the string "hello" needs at least 6 chars to hold it. One for each letter in the string, and another for the 0 null terminator.


So if you need 6 chars to hold that string... how can you hold it in a single char* pointer? The answer is: "you can't". The pointer is not where the string data is stored. The string data is stored elsewhere in memory... and your pointer simply points to the first char of that memory.

Example:

char buffer[6] = "hello"; // <- perfectly legal 

Here we have an array of 6 chars. The array holds our 5 characters of string data + the null terminator.

Now to make that a pointer:

char* ptr = &buffer[0];  // <- ptr now points to the first char in our 'buffer' array 

Again note that 'ptr' does not contain the string data. 'buffer' does. This means that if we change buffer... the data 'ptr' points to will change. Example:

cout << ptr;  // prints "hello" as you'd expect

buffer[0] = 'f';  // change the first character in our buffer

cout << ptr;  // prints "fello" ... even though we didn't modify ptr! 

That is the essence of pointers: they refer to data that exists elsewhere.


Expanding on this... if 'buffer' ceases to exist (as in... it's destroyed, or it goes out of scope)... then our 'ptr' is left "dangling"... ie, it's pointing to data which no longer exists. Dangling pointers are bad and are unsafe to read from/write to. In order to access a pointer, you must make sure it always points to valid data.


So now with that explained... let's understand what memcpy does.

Memcpy takes 2 pointers: a dest pointer, and a source pointer. It will read data from the source pointer and copy it to the dest pointer. It is assumed that both the source and dest pointers are valid and point to real memory that can be safely read/written.

Memcpy does not modify the pointer itself... but rather it modifies the data the pointer points to.



So now let's look at your code:

const char *binaryExt = std::string(".bin").c_str();

Your 'binaryExt' pointer is pointing to string data... but where is this string data? Does it actually exist?

What's happening is you're creating a temp object, pointing to its data, then the temp object immediately dies and your pointer is left dangling. It's similar to this:

const char* binaryExt;
{
    std::string temporary = ".bin";
    binaryExt = temporary.c_str();  // <- points to 'temporary's internal string data.
} // <- 'temporary' is destroyed here... it (along with its string data) no longer exists
// <- so here... 'binaryExt' is left as a dangling pointer.  It points to garbage. 

[vlad from moscow]

Why would not simply to define the pointers as

const char *binaryExt = ".bin";
const char *fileName = "main.cl";

?

[keskiverto]

It is safer to stay in strings:

const std::string fileName{"main.cl"};
std::string binaryFileName;

binaryFileName = fileName;

The binaryFileName has to allocate a memory buffer for the string, and obviously a copy occurs (could even use memcpy, depending how the library implements std::string). There are third-party string classes that share memory buffers, but then have to copy-on-write.

The std::string might appear less efficient than memcpy, but memcpy has its own internal implementation-specific overhead, so not all copy operations are equally good. Ultimately, the piece of code does not look like one that will use 90% of the execution time, and is thus less important to optimize.


A pointer is a variable. While an int variable is a name for integer value that is stored in memory, a pointer variable is a name for a memory address value that is stored in memory. A pointer differs from regular variables by having a dereference operator. Dereferencing a pointer does return a value from the memory address that the pointer variable has.

When wife asks: "Where are my keys?" a value of variable would be "They are on the table". A value of a pointer would be "They are where you did put them". Does that illustrate why pointers are dangerous?

[Disch]

keskiverto wrote:

(could even use memcpy, depending how the library implements std::string)

No. You must never do this.

[andywestken]

(could even use memcpy, depending how the library implements std::string)

=8-@

Andy

[Chervil]

I think the reference to memcpy was a speculation upon how the std::string class is implemented, not a suggestion that we should use memcpy ourselves.

[LB]

Then why even mention it?

[Disch]

The bottom line is that memcpy on any complex type is not safe. Even if one particular implementation of one particular class does not explode when you do it, it's still a bad idea and you should never do it.

In fact.. you probably should just never even use memcpy in C++ anyway since it's type unaware and error prone.