views:

143

answers:

4

I'm trying to allocate a block of memory and then copy data into that space. I made this simple program and it doesn't do what I expect it to do. Could someone please point out my faulty reasoning.

Thanks.

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

void main(void)
{
int t1 = 11;
int t2 = 22;
int *bufptr;

bufptr = calloc(2, sizeof(int));
if(bufptr == NULL)
{
    fprintf(stderr, "Out of memory, exiting\n");
    exit(1);
}

memcpy(bufptr, &t1, sizeof(int));
memcpy((bufptr+sizeof(int)), &t2, sizeof(int));

printf("bufptr11: %d\n", *bufptr);
printf("bufptr22: %d\n", *bufptr+sizeof(int));
}

What it prints out is the following:
bufptr11: 11
bufptr22: 15 (this should be 22 and not 15)

Thanks for all the help everybody, but I just ran into my next snag! The whole point of this exercise is to send some data via udp to another host. I look at the content of the bufptr before I call sendto(), everything looks fine and the sending seems to go well. On the other side (I'm running client/server on 127.0.0.1) I just receive "crap". I call recvfrom(s_fd, bufptr, buflen, etc). I use the same calloc call for allocating memory for bufptr. There is the right amount of data being returned from this call, but the content of it is all just rubbish!

bufptr = calloc(2, sizeof(int));
if(bufptr == NULL)
{
   fprintf(stderr, "Out of memory, exiting\n");
   exit(1);
}

buflen = 2*sizeof(int);

rc = recvfrom(sd, bufptr, buflen, 0, (struct sockaddr *)&serveraddr, &serveraddrlen);
printf("t2: %d\n", *bufptr);
printf("t3: %d\n", *(bufptr+1));
+5  A: 

printf("bufptr22: %d\n", *(bufptr+sizeof(int)));

EDIT: Much more insidious than the parens is the fact (which I obviously missed at first) that you are actually overdoing the pointer arithmetic. The compiler already adjusts bufptr + x to be bufptr + (x * sizeof(int)) bytes. So when you do bufptr + sizeof(int) you are actually overshooting the allocated memory.

You can see this with:

printf("bufptr: %p\n", bufptr);
printf("bufptr + sizeof(int): %p\n", bufptr + sizeof(int));

E.g. for me (32-bit machine) it outputs:

bufptr: 0x876f008
bufptr + sizeof(int): 0x876f018

16 bytes apart, when I only allocated 8 total! A reminder than bufptr[1] is more useful than it looks.

This is the kind of horrible bug that often doesn't manifest at first, then starts causing crashes when you modify unrelated code. valgrind will catch it.

Matthew Flaschen
Very interesting, was not aware of this! Thanks Matthew!
NomadAlien
+2  A: 
printf("bufptr22: %d\n", *bufptr+sizeof(int));

That's being interpreted as (*bufptr) + sizeof(int). Which gives you 11 + 4 to make the 15 you're seeing.

printf("bufptr22: %d\n", *(bufptr+sizeof(int)));

That's what you're after if you're trying to get the 22 back.

rjp
+2  A: 

Try these:

memcpy(bufptr, &t1, sizeof(int)); // copy 11 into the first element of the array.
memcpy(bufptr+1, &t2, sizeof(int)); // bufptr+1 will point to the next int of the array.

printf("bufptr11: %d\n", *bufptr);
printf("bufptr22: %d\n", *(bufptr+1)); // parenthesis needed as * has higher precedence than +
codaddict
Beautiful...Thanks!
NomadAlien
A: 

This:

memcpy((bufptr+sizeof(int)), &t2, sizeof(int));

is wrong, since when you add to a pointer, you add in units of the size of the type the pointer is pointing at. In this case, int. So there's no need to involve sizeof, just use the number of "objects" (ints) directly:

memcpy(bufptr + 1, &t2, sizeof(int));

Also, it's better to use the pointer rather than repeating the type name, this protects you if it should change in the future:

memcpy(bufptr + 1, &t2, sizeof *bufptr);

In the latter case, it's generally a good idea to use the size of the destination pointer, since that protects you (at least a bit) against overwrite if the source pointer type should change. They should of course be the same, but still.

unwind