malloc memory for char array causes access violation

Question

This is probably a noob question. I am using Marmalade SDK. If I allocate some memory for a char array dynamically I get an access violation. I cannot figure out what I am doing wrong. Is this a valid way to use malloc and free?

const char* CClass::caption(int something)
{
    ...

    //char stringPrediction[2000]; // <--- This works

    size_t string01_Size = strlen(string01);
    size_t string02_Size = strlen(string02);

    ...

    stringX = (char *)malloc(string01_Size + string02_Size + 1); // <- Access violation

    strcpy(stringX , string01);
    strcat(stringX , " ");
    strcat(stringX , string02);

    return stringX ;
 }

Destructor:

CClass::~CClass(void)
{
    free(stringX);
}

Then I use it to set the caption of a label on a Button click event

... OnClick...(...)
{
    CClass someObject;
    label.setCaption(someObject.caption());
}

After a few clicks I get access violation.

Unhandled exception at 0x1007ECB8 (s3e_simulator_debug.dll) in 
s3e_simulator_debug.exe:    0xC0000005: Access violation writing
location 0x0000000C.

EDIT: It seems the problems is:

  stringX = (char *)malloc(string01_Size + string02_Size + 1);

I have failed to allocate space for this:

  strcat(stringX , " ");

This should be better:

  stringX = (char *)malloc(string01_Size + 1 + string02_Size + 1);

Can you tell us what's the result of string01_Size + string02_Size + 1, and what's the size of string01 and string02? — user1764961, Jun 12 '13 at 08:12
`Access violation writing location 0x0000000C.` - it seems `malloc()` returned `NULL`. — , Jun 12 '13 at 08:12
@JBL, although C isn't C++, that doesn't mean that function like malloc() shouldn't be used in C++. — user1764961, Jun 12 '13 at 08:21
@H2CO3: OP says he got access violation in line calling `malloc`. — raj raj, Jun 12 '13 at 08:22
@rajraj Although the question is badly phrased, it is clear that it isn't `malloc()` itself that segfaults. Library functions **do not** segfault, they're tested and implemented better than that. — , Jun 12 '13 at 08:23
@user1764961 - thse are the size of string01 and string01; I am trying to concatenate two strings and return the result as char array as caption of a label. — Zingam, Jun 12 '13 at 08:23
@ R. Martinho Fernandes Well, I'll need to convert char array to string and then back to char array, which would not make much sense. — Zingam, Jun 12 '13 at 08:25
@Zingam, I didn't ask for the explanation of these two. I asked for the values of these two. — user1764961, Jun 12 '13 at 08:25
if `string01_Size` and `string02_Size` are valid, you are probably overrunning the array `stringX` in the second call to `strcat` because of an additional space between the concatenated strings. — raj raj, Jun 12 '13 at 08:29
@user1764961 I don't understand the question. They vary. I have I take the values from two arrays defined like that: const char *arr[...] = {"afasdfasdf", "fadfasdfsd"... } — Zingam, Jun 12 '13 at 08:31
@Zingam.. well, just run your code. When it crashes, give us the values (include the strings too). — user1764961, Jun 12 '13 at 08:32
@user1764961 at the moment of the crash the debugger does not display any values at all. All I get is: "no symbos loaded". To make things odd I have replaced the original strings (they seem fine to me) with new for testing and I cannot reproduce the crash. Thank you for trying to help me. — Zingam, Jun 12 '13 at 10:04
@user1764961 I think I found my error. The question remains: is the way I use malloc and free - valid. — Zingam, Jun 12 '13 at 10:17
@Zingam, basically, yes. But, if you call caption() method more than once, then you should check if the buffer has already been allocated. If it is, check if you need to resize the buffer, or not. Also, you probably want to assign NULL to that pointer in the constructor for example. In destructor also check against NULL before calling the free() function. And make sure you don't alter the pointer being returned by the malloc(). I don't know what are you doing, so I can't say anything smart. Just my 2 cents... — user1764961, Jun 12 '13 at 10:23

score 0 · Answer 1 · answered Jun 12 '13 at 08:20

1. You're not testing for the case that malloc fails. The error suggests you're trying to dereference a null pointer - the pointer returned when malloc fails.

2. It doesn't look like you're accounting for the null-byte terminator when calculating the size of your buffer. Assuming string01_Size and string02_Size are the number of characters not including the null byte, you're overflowing your buffer.

Both actions result in undefined behaviour.

score 0 · Answer 2 · answered Jun 12 '13 at 09:31

Access violation encountered during call of malloc or free in most cases indicates that the heap is corrupt. Due to, most commonly overrun or double free or free issued on dangling pointer, happened sometime earlier. May be way earlier.

From the posted code it's not possible to deduce, as it effectively starts with the crash.

To deal with it you may try valgrind, app verifier, other runtime tools helping with corruption. For that one particular issue -- for the general you should not be there in the first place, using malloc, the str* function an all the inherited crap from C.

Consistent use of collection classes, String class, etc would prevent too many cases leading to drastic problems.

score 0 · Answer 3 · answered Jul 03 '13 at 05:35

You've basically spotted the issue: by not allocating for the space you're second strcat will overrun and probably crunch the heap pointers of the next block. Additional things to watch (you're probably be doing this anyway) are to free stringX in CClass::caption() before reallocating so you don't leak. Whatever you really should check that the malloc has not failed before use.

As others have suggested, it may be better to use std::string. You can always convert these to char* if required. Of course, if you do so you should consider that exceptions might be thrown and program accordingly.

malloc memory for char array causes access violation

3 Answers3