3

I have 2 arrays with coordinates and i want to copy them into one array. I used 2 for loops and its working, but i want to know, if i could do it without them, i dont know how to use memcpy in this situation. Here is my code:

int *join(int *first, int *second,int num, int size) {

int *path= NULL;
int i = 0 , j = 0;

path = (int*) malloc (2*size*sizeof(int)); 

    for(i = 0 ; i < num; i++) {
        path[i * 2] = first[i * 2];
        path[i * 2 + 1] = first[i * 2 + 1];

    }

    for(i = num; i < size ; i++) { 
        path[(i*2)] = second[(j+1)*2];
        path[(i*2)+1] = second[(j+1)*2 +1];
        j++;
    }
  return path;
}
8
  • 2
    Please see this discussion on why not to cast the return value of malloc() and family in C.. Commented Dec 2, 2016 at 16:45
  • This path[(i*2)+1] writes to invalid memory with the last iteration. Commented Dec 2, 2016 at 16:48
  • 1
    @JonathanLeffler: The second loop starts from num, num is describing the number of int pairs in first, while size describes the total number of pairs in the result (that is, the number of pairs in first and second combined). The variable names are less than helpful I'll admit. Commented Dec 2, 2016 at 16:54
  • 1
    @alk: How so? They allocated space for 2 * size ints, so valid indices run from 0 to 2 * size - 1 inclusive. On the last iteration, i is size - 1, so i * 2 + 1 is (size - 1) * 2 + 1 which works out to 2 * size - 1. Commented Dec 2, 2016 at 16:57
  • 2
    @ShadowRanger: Hmmm...OK; yes. All the more reason to show how the function is used. A illustrative call like int *new_arr = join(old_1, old_2, num_old_1, num_old_1 + num_old_2) would tell us a lot about how it is supposed to work. Commented Dec 2, 2016 at 16:57

1 Answer 1

4

Just calculate the correct number of bytes to copy and copy from each origin to the correct offset:

int *join(int *first, int *second, int num, int size) {
    // Compute bytes of first
    const size_t sizeof_first = sizeof(*first) * 2U * num;
    // Computes bytes of second as total size minus bytes of first
    const size_t sizeof_second = sizeof(int) * 2U * size - sizeof_first;

    int *path = malloc(sizeof(int) * 2U * size); 

    // Copy bytes of first
    memcpy(path, first, sizeof_first);
    // Copy bytes of second immediately following bytes of first
    memcpy(&path[2U * num], second, sizeof_second);
    return path;
}
Sign up to request clarification or add additional context in comments.

4 Comments

@chux: Oops, I had the correct declaration before, but while editing before posting I lost the type in a copy'n'paste error. :-) Fixed. I also moved the sizeofs first as suggested; it doesn't match OP's ordering, but no reason not to fix it while I'm at it.
thnaks you all for help, that helped a lot
@chux: Heh. I was thinking of that and decided, just to be sure, to make all the 2 literals explicitly unsigned, which I believe will guarantee that &path[2U * num] works. And yeah, I agree that using size_t appropriately is the best solution, but I'm more leery of tweaking the prototype than tweaking the internals.
Agree with concerns about prototype tweaking. 2U * num is a reasonable improvement over 2 * num, though not guaranteed to prevent a computation problem with a large positive num. INT_MAX == UINT_MAX is possible, per C, but likely applies only to esoteric platform in some landfill.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.