ansi-c, better copy array elements to another array?

Question

Hi All, is there anyway to do what I do in Lines 2 and 3, with smth similar to Line 1?

If I just put Line 1, then both "a" and "one.index1" will be pointing to similar location, which I do not want. What I really want is done by Lines 2 and 3. So, is it the only way, or can anyone give better way?

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

typedef struct
{
 int *index1;
} data;

void doo(int *);

int main(int argc, char *argv[])
{
 int *a = (int *) malloc(10*sizeof(int));
 int i;

 for(i=0; i<10; i++)
 {
  a[i] = 2*i;
 }

 doo(a);

 data one;
 //one.index1 = a;                           // Line 1
 /*******************/
 one.index1 = (int *) malloc(5*sizeof(int)); // Line 2
 for(i=0; i<5; i++) one.index1[i] = a[i];    // Line 3
 /*******************/

 printf("%d\n", one.index1[4]);

 free(a);

 printf("%d\n", one.index1[4]);

 free(one.index1);
 return 0;
}

void doo(int *b)
{
 b = (int *) realloc(b, 5*sizeof(int));
 return;
}

Thanks in advance!

Answer 1

You can use memmove. Be sure to #include <string.h> at start.

Replace lines 2 & 3 by,

one.index1 = malloc( 5*sizeof(int) );
memmove(one.index1,a, 5*sizeof(int) );

For example, consider a & c here:

int *a= (int *) malloc(10*sizeof(int));
for(i=0; i<10; i++)
{
    a[i] = 2*i;
}   
int *c= (int *) malloc(10*sizeof(int));
memmove(c,a,10*sizeof(int));

Now both a & c are individual pointers, pointing to "same data" & can be freed separately.

a[0]=0 c[0]=0
a[1]=2 c[1]=2
a[2]=4 c[2]=4
a[3]=6 c[3]=6
a[4]=8 c[4]=8
a[5]=10 c[5]=10
a[6]=12 c[6]=12
a[7]=14 c[7]=14
a[8]=16 c[8]=16
a[9]=18 c[9]=18

Is this new method more efficient? Practically, it is. Compare the time it took for the program to run: (I have used long long type instead of int, to get the noticeable time difference)

Old method:

$ time ./a.out

real    0m0.249s
user    0m0.178s
sys     0m0.070s

New method:

$ time ./a.out

real    0m0.164s
user    0m0.095s
sys     0m0.068s

Answer 2

doo will not work as it is. It would need to be:

void doo(int **b) 
{ 
 *b = (int *) realloc(*b, 5*sizeof(int)); 
 return; 
} 

and called by

 doo(&a);

Your original version may occasionally accidently work, if the realloc occurs immedaitely after the malloc, so there is space to expand the memory block in place. But that should not be counted on.

Now, to answer your actual question, Since we are dealing with simple data items (i.e., ints), you can use memmove() or memcpy() to copy them: (memmove is safe if the memory blocks overlap; memcpy isn't, but that's not a problem here)

  one.index1 = (int *) malloc(5*sizeof(int)); // Line 2 
  memcpy(one.index1, a, sizeof(int) * 5); 

As for the efficency of memmove/memcpy, that's pretty much an unknown area. memmove does a bit more range checking then memcpy, so it'll be a hair slower the memcpy. As far memcpy vs a loop, hard to say. memcpy has a bit more overhead, but it's called a lot, so compiler vendors have a guy who spends a lot of time making sure it's a fast as possible.

Note however, given a small, fixed number of elements to copy, the fastest way to going to be to just copy them directly:

  one.index1 = (int *) malloc(5*sizeof(int)); // Line 2 
  one.index1[0] = a[0];
  one.index1[1] = a[1];
  one.index1[2] = a[2];
  one.index1[3] = a[3];
  one.index1[4] = a[4];

Answer 3

You can use the memmove/memcpy function which is likely to be highly optimized:

const size_t size = 5*sizeof(int);
one.index = (int *) malloc(size);
memcpy(one.index, a, size);

Of course, you can wrap this into a function:

int *getCopy(const int *source, size_t count)
{
  int *result = (int *)malloc(count * sizeof(int));
  if (!result)
    return NULL;
  memcpy(result, source, count * sizeof(int));
  return result;
}

// Usage
one.index = getCopy(a, 5);

Answer 4

It depends on what specifically you want to avoid about lines 2 ond 3. If you want to avoid the loop, use memcpy, although most compilers should replace your loop with code equivalent to a memcpy call. If you want a one-liner operation, you can write a static function that takes the address of one.index1, the size of the array a, and a as arguments.

int * f (int **int_pp, size_t len, int const *a) {
    *int_pp = (int *) malloc (len * sizeof (int));
    if ( *int_pp != NULL ) {
       memcpy (*int_pp, a, len);
    }
    return *int_pp;
}

Don't forget to check if malloc returns NULL!

If you want to avoid the call to malloc, I think the only option would be to have a chunk of memory set aside that you manage manually. It's almost never worth it to try to manage memory yourself, though.

Answer 5

Some quibbles.

Life's easier if you define functions before they're called (if they're in the same source file, that is); that way you don't need to worry about keeping declarations/definitions in sync.

Don't cast the result of malloc(). As of C89, you don't need to, and doing so may suppress a useful diagnostic if you forget to #include stdlib.h or otherwise don't have a prototype for malloc() in scope (although newer compilers are catching this error even with the cast since it happens so bloody often).

Use the sizeof operator on the thing you're allocating, not a type; again, it makes life simpler since you don't have to keep the declaration and malloc() calls in sync.

If you want a function to modify a pointer argument, you must pass a pointer to that pointer:

void doo(int **p)
{
  *p = realloc(*p, 5 * sizeof **p); 
}

Otherwise the change to the formal parameter p is not reflected in the actual parameter.

Life's also easier if you abstract out your memory management functions; you can replace lines 2 and 3 with a single function call:

int *copy(int * const source, size_t len)
{
  int *dest = malloc(len * sizeof *dest);
  if (dest)
    memcpy(dest, source, len * sizeof *source);
  return dest;
}

which you call as

one.index1 = copy(a, len);