c++ unsigned char array allocation - segmentation fault

Question

I can't figure out what seems to be the problem that I get a segmentation fault from this:

#include <stdio.h>
#include <stdlib.h>
#include <cstring>

void alloc(unsigned char *data) {
    data = (unsigned char *) malloc(20);
    memset(data, 0, 20);
}

void main() {
    unsigned char *data = NULL;
    int i;
    alloc(data);
    for (i = 0; i < 20; i++) {
        data[i] = i;
        printf("%d ", *(data + i));
    }
    free(data);
}

Unsigned char is 1 byte so the loop through 20 should be correct

You need to pass unsigned char** data to the alloc function and then use *data = (unsigned char *) malloc(20); or like Baum mit Augen said use a reference to the pointer. — Hauke S, Jun 08 '15 at 15:09
If you try to learn C++ like this, you need to throw your current book / tutorial away and get a [better one](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — Baum mit Augen, Jun 08 '15 at 15:09
Well, looking at it again it's C++ since he's using ``... Otherwise it looks completely like C — AliciaBytes, Jun 08 '15 at 15:10
If you used a debugger (or even print statements), you should have seen that `data` was not changed after the call to `alloc`, and that it is still `NULL`. That should have given a clue as to what is happening wrt passing-by-value and temporaries. This is why debugging your code is important -- so that you 1) don't waste time asking on SO because you would have known the reason for the issue and/or 2) You would be able to ask a more focused question such as "why hasn't my pointer changed?" — PaulMcKenzie, Jun 08 '15 at 15:11
Rolled back the changes bc OP specifically asked about C++ both in title and in tags, and this is not C because ``. This edit conflicted with the author's intend. — Baum mit Augen, Jun 08 '15 at 15:27
^^ what @PaulMcKenzie says. Questions that indicate that no debugging was done should be closed as ... something. — Martin James, Jun 08 '15 at 15:56
I'm voting to close this question as off-topic because a trivial debugging effort would have revealed the bug. — Martin James, Jun 08 '15 at 15:59

score 7 · Answer 1 · answered Jun 08 '15 at 15:08

7

void alloc(unsigned char *data) {
    data = (unsigned char *) malloc(20);
    memset(data, 0, 20);
}

modifies a local copy of your pointer. You could pass it by reference to make your example work:

void alloc(unsigned char *&data) {
    data = (unsigned char *) malloc(20);
    memset(data, 0, 20);
}

answered Jun 08 '15 at 15:08

Baum mit Augen

49,044
25
144
182

if the alloc() function argument is a pointer, shouldn't it be passed by reference by default? – Ioan Fulgeanu Jun 08 '15 at 15:11
@IoanFulgeanu everything in C++ (except for plain C arrays that decay into pointers) is pass by value by default, even pointers. – AliciaBytes Jun 08 '15 at 15:12
@IoanFulgeanu No, the pointer itself still gets passed by value. You seem to be confused because you can use the copy of the pointer to access whatever it points to, which then is the same object the pointer you gave to the function points to. – Baum mit Augen Jun 08 '15 at 15:13

Vlad from Moscow · Accepted Answer · 2015-06-08T15:33:06.920

It seems that your program is written in C instead of C++. In C++ you should use operator new [] instead of malloc.

The problem with the function is that function parameters are its local variables. So the function parameter char *data is a copy of its argument declared in main like

unsigned char *data = NULL;

So any changes of the parameter in function alloc do not influence on the original argument. After exiting the function the parameter (local variable) will be destroyed and the original variable in main will not be changed.

You have two approaches. Either you declare the function the following way

void alloc(unsigned char **data) {
    *data = (unsigned char *) malloc( 20 );
    if ( *data ) memset( *data, 0, 20 );
}

and call it like

alloc( &data );

Or you declare the function the following way

unsigned char * alloc() {
    unsigned char *data = (unsigned char *) malloc( 20 );

    if ( data ) memset( data, 0, 20 );

    return data;
}

and call it in main like

data = alloc();

Take into account that function main shall be decalred in C like

int main( void )
^^^

and in C++ like

int main()
^^^

score 0 · Answer 3 · edited May 23 '17 at 12:08

You could also return the pointer, rather than passing it to the function.

unsigned char *alloc() {
    unsigned char *data = (unsigned char *) malloc(20);
    memset(data, 0, 20);
    return data;
}

And then,

...
unsigned char *data = alloc();
int i;
...

You tagged C++, but your code looks like C. If you really are trying to write C++ you shouldn't be using malloc. Either use new[] or the appropriate smart pointers.

EDIT: You're using void main(). Don't do that. What should main() return in C and C++?

c++ unsigned char array allocation - segmentation fault

3 Answers3