How to copy with memcpy?

Question

char *concat(char *num1, const char *num2, int index) {

    int length1 = strlen(num1);
    int length2 = strlen(num2); 
    int lengthNum = 0;                   
    char *num = malloc(length1 + length2 + 1);

    if (num == NULL) {
        free(num);
        return NULL;
    }
    // memcpy(num, num1, length1);
    // memcpy(num + length1, num + index, length2 + 1);    

    for (int i = 0; i < length1; i++) {
        num[lengthNum] = num1[i];
        lengthNum++;
    }
    for (int i = index; i < length2; i++) {
        num[lengthNum] = num2[i];
        lengthNum++;
    }
    return num;
}

I tried to use memcpy, but than my program doesn't work correctly (copies wrongly, but valgrind doesn't show an error).

But when I use two for loops instead, it works properly, but than valgrind shows an error

uninitialised value was created by a heap allocation.

How to use properly memcpy in this case?

What is the purpose of `int index`? You are allocating for the whole of both strings (and a terminator). — Weather Vane, May 02 '22 at 15:27
This `index` is likely breaking things. If it is not zero, it won't work correctly. — Eugene Sh., May 02 '22 at 15:28
https://stackoverflow.com/questions/5587121/how-to-properly-use-memcpy — Avinash, May 02 '22 at 15:29
I will copy the first part (before index 'index') and the second part (from num2) begining (starting) from index 'index'. Like a have numbers (char*) 1234 and 5678 and 'index'==2, I will (want to) receive and return in this function a number (char*) 1278 @WeatherVane — Катя Павліченко, May 02 '22 at 15:32
That is not what the loop code does. BTW you forgot the terminator `num[length1 + length2] = '\0';` — Weather Vane, May 02 '22 at 15:35
Sorry, I`ve made a mistake, I want to copy the whole first part and the second part (from num2) starting from index 'index'. @WeatherVane — Катя Павліченко, May 02 '22 at 15:37
You were copying from `num` instead of `num2`. Try `memcpy(num + length1, num2 + index, length2 - index + 1);` which includes the terminator. But first check that `index <= length2`. — Weather Vane, May 02 '22 at 15:40
Since you use `strlen` on both `num1` and `num2`, you should either use `strcpy` and `strcat`, or you should `snprintf`. Using `memcpy` is not wrong, but it's strange to use if you know them to be strings. — Cheatah, May 02 '22 at 16:00
Can you clarify: does `concat("1234", "5678", 2)` result in `"1278"`, or `"123478"`? — Oka, May 02 '22 at 16:08

score 0 · Answer 1 · answered May 02 '22 at 15:59

0

memcpy(num, num1, length1);
memcpy(num + length1, num2, length2 + 1);

answered May 02 '22 at 15:59

zzz3265

11
1

chqrlie · Answer 2 · 2022-05-02T21:38:00.943

Your program has multiple issues:

freeing a null pointer when malloc failed is useless (but harmless).
you allocate length1 + length2 + 1 bytes, which is most likely too large as you intend to copy from index in the second string.
you should use type size_t for the lengths and offsets.
you copy from num2 + index without checking that index is a valid offset inside the string num2.
you should set a null terminator at the end of the new string.

Here is a modified version:

#include <stdlib.h>
#include <string.h>

char *concat(const char *num1, const char *num2, size_t index) {
    size_t length1 = strlen(num1);

    /* skip index bytes in num2 */
    while (index --> 0 && *num2)
        num2++;

    size_t length2 = strlen(num2);
    char *num = malloc(length1 + length2 + 1);

    if (num != NULL) {
        size_t j = 0;

        while (*num1) {
            num[j++] = *num1++;
        }
        while (*num2) {
            num[j++] = *num2++;
        }
        num[j] = '\0';
    }
    return num;
}

and using memcpy:

#include <stdlib.h>
#include <string.h>

char *concat(const char *num1, const char *num2, size_t index) {
    size_t length1 = strlen(num1);
    /* skip index bytes in num2 */
    while (index --> 0 && *num2)
        num2++;
    size_t length2 = strlen(num2);
    char *num = malloc(length1 + length2 + 1);
    if (num != NULL) {
        memcpy(num, num1, length1);
        memcpy(num + length1, num2, length2 + 1);    
    }
    return num;
}

concat returns a pointer to an allocation array. It is the responsibilty of the caller to free this object after use. For example:

#include <stdio.h>

int main() {
    char *p = concat("Hello", "dear world", 4);
    if (p != NULL) {
        printf("%s\n", p);
        free(p);
    }
    return 0;
}

To get in front of the probable follow up question: [What is the "-->" operator in C/C++?](https://stackoverflow.com/q/1642028/2505965) — Oka, May 02 '22 at 17:57
Because after all vilgrind writes that ```3 bytes in 1 blocks are definitely lost in loss record 1 of 1 ==121360== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==121360== by 0x109CDD: concat (in /home/kate/Pulpit/maain) ==121360== by 0x10A0F8: phfwdGet (in /home/kate/Pulpit/maain) ==121360== by 0x10A2FD: main (in /home/kate/Pulpit/maain)``` — Катя Павліченко, May 02 '22 at 18:30

How to copy with memcpy?

2 Answers2