CODEWITHSUNDEEP
cmemcpy
user1692342
user1692342

Reputation: 5237

memcpy crashing after multiple runs

I had written erroneous piece of code, which crashed only after multiple runs in release mode in visual studio (greater than 50). Can someone explain why this piece of code didn't crash much earlier

char *pcBuffer= "Some Text";
char *pctempBuff = NULL;

    pctempBuff = malloc(100);
    memset(pctempBuff,0,100);
    memcpy(pctempBuff,pcBuffer,100);

The above code crashed after multiple runs.

I corrected it to the following code which is correct and it no longer crashes

char *pcBuffer= "Some Text";
char *pctempBuff = NULL;

pctempBuff = malloc(strlen(pcBuffer)+1);
memset(pctempBuff,0,strlen(pcBuffer)+1);
memcpy(pctempBuff,pcBuffer,strlen(pcBuffer)+1);

Upvotes: 0

Views: 1362

Answers (2)

simonc
simonc

Reputation: 42165

There are two errors in your initial code.

malloc can fail, returning NULL in low memory. If you keep allocating memory without freeing any, the system will eventually run out of memory and malloc will return NULL. You need to test for this

pctempBuff = malloc(100);
if (pctempBuff != NULL) {
    memset(pctempBuff,0,100);
    memcpy(pctempBuff,pcBuffer,strlen(pcBuffer)+1);
}

You were also reading memory you didn't own by telling memcpy to copy 100 bytes from the address of the 10 byte pcBuffer. This results in undefined behaviour. A crash after many apparently successful iterations would be a possible (if unlikely) instance of this. Your second example is correct because it only reads the memory for pcBuffer.

Since you're copying a string, you could do this more clearly/easily/safely by using strcpy

pctempBuff = malloc(100);
if (pctempBuff != NULL) {
    strcpy(pctempBuff,pcBuffer);
}

Upvotes: 3

Shafik Yaghmour
Shafik Yaghmour

Reputation: 158449

You are reading past the memory allocated to pcBuffer in the first case, this is undefined behavior, you can make no prediction about the behavior of such a program and in fact it can act well behaved for a long time and may never crash.

This is the most obvious error, you also need to check that malloc does not fail, but since this is only a sample from larger code it is unclear if that is a real problem or just left out code.

Upvotes: 0

Related Questions