CODEWITHSUNDEEP
clinuxsegmentation-faultmemcpy
ogs
ogs

Reputation: 1239

memcpy unsigned int to unsigned char segmentation fault

I would like to copy 4 bytes from unsigned int to unsigned char array. Once executed the following function get_result goes to segmentation fault :

int exec_cmd(unsigned int * apu32Var)
{
    int  ret = -1;
    char cmd[100] = { 0 };
    char resp[100] = { 0 };

    sprintf(cmd, "%s %s", "/home/send_frames.sh", "read");
    ret = exec_cmd_ret_result(cmd, resp);

    if( apu32Var != NULL )
    {
        *apu32Var = (((unsigned int)resp[0]) <<24)+(((unsigned int)resp[1]) <<16)+(((unsigned int)resp[2]) <<8)+(unsigned int)resp[3];
    }
    return ret;
}

int get_result(unsigned char * buffer, unsigned short * size)
{
    unsigned int u32Var = 0;

    exec_cmd(&u32Var);

    memcpy(buffer, &u32Var, sizeof(unsigned int));  
    *size += sizeof(unsigned int);
    return 0;
}


int main(int argc, char **argv)
{
    unsigned char *buf;
    unsigned short *size;

    get_result(buf+4, size);

    return 0;
}

However, regarding to memcpy() man page it seems memcpy() are well managed. What is going wrong ?

Upvotes: 1

Views: 1585

Answers (2)

dbush
dbush

Reputation: 225387

buf in main is never initialized, so it points to some random location in memory. This is undefined behavior and a perfect recipe for a segfault.

Similarly, *size is read from when you use +=, but the value was never initialized in main, so your dereference an undefined value.

You should declare buf as an array of sufficient size and pass that in. Also, declare size as an int, initialize it to 0, and pass its address:

int main(int argc, char **argv)
{
    unsigned char buf[100];
    unsigned short size = 0;

    // I'm assuming this was a typo and you ment to call get_result instead of test_result
    get_result(buf, &size);

    return 0;
}

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409462

Assuming your call to test_result actually should be calling get_result, then you have two big problems.

The first and most serious is that you pass in uninitialized local variables as arguments to the function. Uninitialized local variables have indeterminate values. For a pointer, it means it can point just about anywhere, and trying to dereference it will lead to undefined behavior. You need to actually make these pointers point somewhere valid for it to work. This goes for both variables.

The second problem is that you misunderstand how emulating pass by reference works in C. Yes the function should take a pointer, but you should not actually create a pointer variable and pass to the function. Instead you should use the address-of operator & on a non-pointer variable.

To solve both problems, your code should look something like

unsigned char buf[256] = { 0 };  // Arbitrary size, all initialized to zero
unsigned short size = 0; // To make sure it's properly initialized

get_result(buf + 4, &size);  // Note use of & to pass a pointer to the variable size

Note that it works using an array, since arrays naturally decays to pointers to its first element.

Upvotes: 2

Related Questions